豆豆友情提示:这是一个非官方 GitHub 代理镜像,主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content

Inject the file_parser and use its attributes, Add a prepare! hook#13208

Merged
brrygrdn merged 3 commits intomainfrom
brrygrdn/dg-7930-parser-is-internal-to-grapher
Oct 2, 2025
Merged

Inject the file_parser and use its attributes, Add a prepare! hook#13208
brrygrdn merged 3 commits intomainfrom
brrygrdn/dg-7930-parser-is-internal-to-grapher

Conversation

@brrygrdn
Copy link
Copy Markdown
Contributor

@brrygrdn brrygrdn commented Oct 1, 2025

What are you trying to accomplish?

This PR makes a slight adjustment to the DependencyGrapher component to have it accept the file parser directly and execute it via a hook that ecosystems can redefine in the event they need to add before/after logic or an alternative parsing strategy in rare cases.

Since the FileParser's attributes contains the dependency files, credentials, etc, they can provide the grapher everything it needs to use native helpers to make additional calls without having to change the interface in future.

Anything you want to highlight for special attention from reviewers?

Rather than get into metaprogramming to instantiate the parser within the grapher, I elected for a simple DI approach to give us the prepare! method as a call site ecosystems can override in whatever way they need to.

This fulfils the goal of the grapher being the component which holds unto any logic that is redundant to parsing in update job cases without adding a lot of complexity - in the worst case scenario where an ecosystem needs a totally different parsing strategy for graphing, we can easily use the attributes of the base parser to create a new special-case one on the fly.

How will you know you've accomplished your goal?

Nothing breaks, this is a small structural refactor with no behaviour changes.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: go:modules Golang modules labels Oct 1, 2025
@brrygrdn brrygrdn force-pushed the brrygrdn/dg-7930-parser-is-internal-to-grapher branch 2 times, most recently from 50d1d25 to d39414b Compare October 1, 2025 15:32
@brrygrdn brrygrdn marked this pull request as ready for review October 1, 2025 15:32
@brrygrdn brrygrdn requested a review from a team as a code owner October 1, 2025 15:32
Ahmed3lmallah
Ahmed3lmallah previously approved these changes Oct 1, 2025
Copy link
Copy Markdown
Contributor

@Ahmed3lmallah Ahmed3lmallah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a minor comment.

Comment thread common/lib/dependabot/dependency_graphers/base.rb Outdated
@brrygrdn brrygrdn force-pushed the brrygrdn/dg-7930-parser-is-internal-to-grapher branch from d39414b to 8020584 Compare October 1, 2025 17:26
@brrygrdn brrygrdn requested a review from Ahmed3lmallah October 1, 2025 17:28
Copy link
Copy Markdown
Contributor

@Ahmed3lmallah Ahmed3lmallah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing the comment!

@brrygrdn brrygrdn force-pushed the brrygrdn/dg-7930-parser-is-internal-to-grapher branch from 255f5ed to 723b6df Compare October 2, 2025 11:42
@brrygrdn brrygrdn merged commit 76dfb83 into main Oct 2, 2025
177 of 180 checks passed
@brrygrdn brrygrdn deleted the brrygrdn/dg-7930-parser-is-internal-to-grapher branch October 2, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: go:modules Golang modules L: ruby:bundler RubyGems via bundler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants