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

Revert "Support retrieving workspace of path dependencies in cargo (#10550)" #10599

Merged

Conversation

Jefffrey
Copy link
Contributor

@Jefffrey Jefffrey commented Sep 13, 2024

What are you trying to accomplish?

This reverts commit f238da5.

Closes #10584

#10550 caused a regression, reverting this for now as it was new functionality. Will need to revisit later after adding more test cases based on the repositories that encountered regressions due to this PR.

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

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.

@Jefffrey Jefffrey requested a review from a team as a code owner September 13, 2024 09:12
@github-actions github-actions bot added the L: rust:cargo Rust crates via cargo label Sep 13, 2024
@Jefffrey
Copy link
Contributor Author

An alternative could be to remove these error conditions entirely:

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg

Which I think would keep this new functionality without affecting old behaviour

@thavaahariharangit thavaahariharangit merged commit 902c1da into dependabot:main Sep 13, 2024
39 checks passed
@abdulapopoola
Copy link
Member

An alternative could be to remove these error conditions entirely:

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg

Which I think would keep this new functionality without affecting old behaviour

Thanks @Jefffrey , please if you don't mind, can you raise this as a brand new PR?

@Jefffrey Jefffrey deleted the revert-cargo-path-dependency-find branch September 18, 2024 10:30
Jefffrey added a commit to Jefffrey/dependabot-core that referenced this pull request Sep 18, 2024
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
Development

Successfully merging this pull request may close these issues.

"Could not resolve workspace root for path dependency" when crate is one directory under workspace root
3 participants