Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cd: correctly find workspace.dependencies-declared dependencies and their Cargo.toml #7649

Merged

Conversation

ggawryal
Copy link
Contributor

I've decided to pick up #5865 as fixing the issue is quite urgent for my project. Added some more code to fetch [workspace.dependencies] from the root manifest, because extending required_path? function was not sufficient. Added a test as well - previously running dependabot on the repository I've attached to the test was finishing with a failure.

Closes #5864.

@ggawryal ggawryal requested a review from a team as a code owner July 27, 2023 12:16
@github-actions github-actions bot added the L: rust:cargo Rust crates via cargo label Jul 27, 2023
@ggawryal ggawryal changed the title 5864 fix rust cargo workspace dep discovery cd: correctly find workspace.dependencies-declared dependencies and their Cargo.toml Jul 27, 2023
@jeffwidman
Copy link
Member

Thanks for picking this up!

Added some more code to fetch [workspace.dependencies] from the root manifest, because extending required_path? function was not sufficient

Ah, that explains why I kept running into problems when I'd tried to write the test myself.

I'm about to walk out the door to go camping, so hopefully someone else on the team can take a look... but if not, I'm happy to take a look next week. If you don't hear from someone by Wednesday, feel free to leave a comment and tag me by name as a reminder.

Copy link
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

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

This looks great!

I'm not sure why the one Updater test is failing but it's not related to these changes.

I also looked at the test repo and although it differs slightly from what's described in #5864, it looks correct.

Also, the requests to the test fixtures are all stubbed out so it's okay to reference the test repo 👍🏾

@Nishnha Nishnha merged commit ef7e872 into dependabot:main Jul 27, 2023
90 checks passed
@Nishnha
Copy link
Member

Nishnha commented Jul 27, 2023

This is now in production.

@ggawryal
Copy link
Contributor Author

That's a great news, many thanks for your support!

@abdulapopoola
Copy link
Member

Thanks for helping to make :dependabot: better @ggawryal !

brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
…workspace-dep-discovery

cd: correctly find workspace.dependencies-declared dependencies and their Cargo.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: rust:cargo Rust crates via cargo
Projects
None yet
5 participants