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

Be sure to select packages only from the workspace, not other dependencies #237

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

maackle
Copy link
Contributor

@maackle maackle commented Jun 7, 2024

In situations where multiple version of a package are included as dependencies, cargo-rdme only chooses the first one it finds. metadata.workspace_members seems to only list the latest version. This PR updates the package selection to choose the package with the latest version when there are multiple choices.

The test in the first commit fails, the second commit fixes it.

@maackle
Copy link
Contributor Author

maackle commented Jun 7, 2024

This may not be the ideal solution (ideally we select the local package, if there are multiple), but I think this is better than selecting arbitrarily.

@orium
Copy link
Owner

orium commented Jun 10, 2024

This may not be the ideal solution (ideally we select the local package, if there are multiple), but I think this is better than selecting arbitrarily.

We can do better. What we want to do is select the package that is in the workspace. That can be done by checking the package id. The method select_package() can simply become

metadata.packages.iter().find(|package| {
    package.name == package_name && metadata.workspace_members.contains(&package.id)
})

Edit: Can you also squash your commits. No need to have a commit with the test before the fix.

@maackle
Copy link
Contributor Author

maackle commented Jun 10, 2024

Ah yes that's better. I made that change and reset everything to a single commit.

@maackle maackle changed the title Select package with latest version, not just first match Be sure to select packages only from the workspace, not other dependencies Jun 10, 2024
@orium orium merged commit b7ce47e into orium:main Jun 10, 2024
4 checks passed
@orium
Copy link
Owner

orium commented Jun 10, 2024

Thanks for you contribution @maackle. I'll cut a release now.

@orium
Copy link
Owner

orium commented Jun 10, 2024

Released in v1.4.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants