Add GitHub file links with line numbers to Slack messages #114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enhancement improves the developer experience by making TODO notifications more actionable: - Capture line numbers from Prism comment locations during parsing - Add GitUtils module to detect GitHub repositories and generate file links - Enhance Slack messages with clickable GitHub links (format: https://github.com/org/repo/blob/branch/file.rb#L42) - Provide readable fallback for non-git environments (e.g., "on line 42") When a git repository is detected, Slack messages now include clickable links that take developers directly to the exact line in GitHub. For non-git environments, the message displays a human-readable line reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Updated OutputTest to verify GitHub link generation works in git repos - Added test for non-git fallback behavior showing readable line numbers - Created GitUtilsTest with 10 test cases covering: - HTTPS and SSH GitHub URL parsing - Non-GitHub repository handling - Branch detection and link generation - Absolute vs relative path handling - Edge cases (no git repo, non-GitHub remotes) All tests pass (186 runs, 434 assertions, 0 failures). Fixes #66 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Changed `return nil` to `return` (Style/ReturnNil) - Added parentheses to test assertions (Style/MethodCallWithArgsParentheses) - Updated comments to use inclusive language (Naming/InclusiveLanguage) All RuboCop checks now pass (39 files, 0 offenses). All tests still passing (186 runs, 434 assertions, 0 failures). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This addresses the issue where smart_todo might be run from a different directory than the git repository root (e.g., from a parent directory or in CI environments with non-standard working directories). **Changes:** 1. **Added `find_git_root` method to GitUtils** - Walks up the directory tree from a file path to find `.git` directory - Handles both file paths and directory paths - Returns nil if no git repository is found 2. **Added caching for performance** - `git_root_cache` caches git root lookups per directory - `git_info_cache` caches GitHub info per repository - Thread-safe cache implementation using accessor methods 3. **Updated Base dispatcher** - Now uses `find_git_root(@file)` instead of `Dir.pwd` or `base_path` - Correctly detects git repo from the actual file being processed - Works regardless of where smart_todo command is run from 4. **Added comprehensive tests** - 6 new tests for `find_git_root` functionality - Tests cover: repo root, subdirectories, file paths, deeply nested paths - Tests verify caching behavior and nil returns for non-git directories - Updated OutputTest to properly test non-git fallback with tmpdir **Benefits:** - Works correctly when run from parent directories - Works in CI environments with non-standard working directories - Handles absolute and relative file paths correctly - Efficient with caching to avoid repeated filesystem traversals - No breaking changes to API or CLI All tests passing (192 runs, 442 assertions, 0 failures). RuboCop clean (39 files, 0 offenses). Fixes the concern raised in #94 (comment) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
human tldr: I wanted line numbers in our slack messages. This adds it.
Potentially this also adds github links, but I saw the comment here and it's possible that this won't work- tests seem to be fine but I'd like somebody who understands the issue better to weigh in on this code
The rest is summarised by Claude, seems good and i babysat him for this, read all code with my own human eyeballs etc.
Summary
This PR enhances Slack notifications to include clickable GitHub links with line numbers, making TODO notifications more actionable.
Fixes #66
Changes:
GitUtilsmodule to detect GitHub repositories and generate file linkshttps://github.com/Shopify/smart_todo/blob/main/file.rb#L42)Examples
With GitHub repository:
Creates a clickable link in Slack that navigates directly to the line in GitHub
Without GitHub repository:
Falls back to human-readable line reference
Key Technical Improvement
Git Repository Detection Fix
The PR includes an important fix for how git repositories are detected. Previously, the code would use
Dir.pwd(the current working directory) to look for.git, which could fail when:Solution: The new
find_git_rootmethod walks up the directory tree from the actual file being processed to find the.gitdirectory. This ensures correct behavior regardless of where the command is run from.This addresses the concern raised in #94 (comment) about
base_pathpotentially being incorrect.Test Coverage
Updated Tests
OutputTest#test_dispatch_with_github_link- Verifies GitHub links in git reposOutputTest#test_dispatch_without_github_link- Verifies readable fallback formatNew Tests (GitUtilsTest)
find_git_rootwalking up directory tree from files/subdirectoriesAll tests passing: 192 runs, 442 assertions, 0 failures
Performance
The implementation includes caching for git repository information:
git_root_cache- Caches git root lookups per directorygit_info_cache- Caches GitHub repository infoThis ensures efficient operation even when processing hundreds of files, as each directory is only scanned once.
🤖 Generated with Claude Code