Add deep links to GitHub/Buildkite in Slack messages #120
Conversation
sambostock
force-pushed
the
slack-deep-links
branch
3 times, most recently
from
d4a91a4 to
74939a8
Compare
sambostock
force-pushed
the
slack-deep-links
branch
2 times, most recently
from
4b13c11 to
3bc8d3c
Compare
| # @param todo [SmartTodo::Todo] the todo containing filepath and line numbers | ||
| # @return [Link, nil] Link with url and display, or nil if no link can be generated | ||
| def for_todo(todo) | ||
| return if UNLINKABLE_PATHS.include?(todo.filepath) |
There was a problem hiding this comment.
Is this required ? I don't think we run smart todo with inline code on CI (it wouldn't make a lot of sense)
There was a problem hiding this comment.
Good question! While you're right that the CLI iterates over actual files, the library's API explicitly defaults to -e to support programmatic use cases:
CommentParser.parse(source, filepath = "-e")Todo.new(source, filepath = "-e")
Without the check, we'd generate broken URLs like https://github.com/org/repo/blob/sha/-e#L1. The check ensures graceful fallback to `-e:1` (code-formatted path) instead of a broken link.
| relative_to_workspace(Dir.pwd, ENV["BUILDKITE_BUILD_CHECKOUT_PATH"]) | ||
| end | ||
| relative_path = join_path(prefix, todo.filepath.delete_prefix("./")) | ||
| repo = ENV["BUILDKITE_REPO"].delete_suffix(".git") |
There was a problem hiding this comment.
Great idea to grab env from the CI provider. Do you think the --repo option we pass to the CLI is now any useful ? If its still useful, should it have precedence ?
There was a problem hiding this comment.
These features are actually complementary rather than overlapping:
| Feature | Purpose | Output |
|---|---|---|
--repo |
Human-readable repo name | "in repository \smart_todo`"` at end of message |
DeepLink |
Clickable URL | <url|file.rb:42> for file reference |
Both can appear in the same message. --repo remains useful for:
- Local runs — No CI env vars means
DeepLinkreturnsnil, but--repostill provides context - Unsupported CI providers — Jenkins, GitLab CI, CircleCI, etc. won't have the expected env vars
- Custom naming — In monorepos, someone might want a different name than the GitHub repo
No precedence logic needed since they operate on different parts of the message.
sambostock
force-pushed
the
slack-deep-links
branch
from
df6904d to
9c61f72
Compare
When running in CI (GitHub Actions or Buildkite), TODO notifications now include clickable links to the exact file and line in the source repository. Features: - Detects CI environment via GITHUB_ACTIONS or BUILDKITE env vars - Builds URLs using repository, commit SHA, and file path - Supports line ranges for multi-line TODOs (e.g., #L42-L45) - Handles monorepo subdirectories via workspace-relative paths - Falls back gracefully when not in a supported CI environment Also includes: - Optional line_number parameter for Todo with deprecation warning - Backward compatibility for Todo instances without line_number Co-Authored-By: Claude Opus 4.5 <[email protected]>
sambostock
force-pushed
the
slack-deep-links
branch
from
9c61f72 to
4240314
Compare
Edouard-chin
left a comment
Edouard-chin
left a comment
There was a problem hiding this comment.
👍
Maybe @bashu-shopify has some feedback as he implemented a similar feature in #114 , but the approach in this PR with CI env variables is better.
2 participants
./lib/smart_todo.rbfile.lib/smart_todo.rb:7.Summary
File references in Slack messages now link to GitHub when running in CI (GitHub Actions or Buildkite).
GITHUB_ACTIONSorBUILDKITEenv varsfile.rb:42-45)SMART_TODO_LINK_FORMATenv varSMART_TODO_REPO_PATHenv varTest plan
🤖 Generated with Claude Code