Extract pre-commit dependency version from comment in PR description#14403
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an extension point in Dependabot::Dependency to let ecosystems customize how humanized_previous_version is rendered, and uses it in the pre_commit ecosystem to display comment-pinned versions (e.g., # v2.2.1) instead of raw SHAs in PR descriptions.
Changes:
- Add a per-ecosystem registration mechanism for
humanized_previous_versionformatting inDependabot::Dependency. - Register a
pre_commitbuilder that extracts a version from therev:comment. - Add specs covering the new registration behavior and the
pre_commitextraction behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
common/lib/dependabot/dependency.rb |
Adds builder registry + refactors humanized_previous_version into default/custom paths. |
common/spec/dependabot/dependency_spec.rb |
Adds unit tests for builder registration and fallback behavior. |
pre_commit/lib/dependabot/pre_commit.rb |
Registers the pre_commit humanized-previous-version builder using comment parsing. |
pre_commit/spec/dependabot/pre_commit_spec.rb |
Adds pre_commit-focused specs for extracting version from comment metadata. |
.rubocop_todo.yml |
Adds a new Metrics/ClassLength exclusion for Dependency. |
| Metrics/ClassLength: | ||
| Exclude: | ||
| - 'common/lib/dependabot/dependency.rb' | ||
| - 'opentofu/lib/dependabot/opentofu/file_parser.rb' |
There was a problem hiding this comment.
Adding common/lib/dependabot/dependency.rb to the global RuboCop todo exclusions (Metrics/ClassLength) increases long-term lint debt. Since the class-length increase here is driven by the new humanized_previous_version refactor, it would be better to adjust the implementation (e.g., keep the existing logic inline, or extract functionality into a separate helper/module) so the file remains compliant without extending .rubocop_todo.yml exclusions.
|
Looks good to me except the suggestion made https://github.com/dependabot/dependabot-core/pull/14403/changes#r2908144625. If you consider that it will be great. Just was thinking: Not that important |
@kbukum1 the new functionality added in the |
b60c3f2 to
080d617
Compare
080d617 to
f3bd8ca
Compare
What are you trying to accomplish?
Fixes the issue where PR descriptions showed raw commit hashes instead of the version from comments when using frozen SHA format in
.pre-commit-config.yaml.Problem:
When users pin pre-commit hooks using SHA with a version comment:
The PR description incorrectly displayed:
Instead of the expected:
Solution:
Added a registration mechanism for ecosystems to customize how
humanized_previous_versionis computed, then registered a builder forpre_committhat extracts the version from the comment metadata.Anything you want to highlight for special attention from reviewers?
Changes:
dependency.rb:@humanized_previous_version_buildersclass variableregister_humanized_previous_version_builderandhumanized_previous_version_builder_for_package_managerclass methodshumanized_previous_versionto check for a registered builder before falling back to default behaviorpre_commit.rb:humanized_previous_version_builderthat extracts versions from comments using COMMENT_VERSION_PATTERNHow will you know you've accomplished your goal?
If the PR description correctly shows the extracted version from comment.
Checklist