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

support cargo's --manifest-path #146

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

PhilippMolitor
Copy link
Contributor

self-explanatory. issue raised in #95

@PhilippMolitor PhilippMolitor requested a review from a team as a code owner October 14, 2024 09:33
@burrbull
Copy link
Member

Could you fix CI/clippy errors?

@jannic
Copy link
Member

jannic commented Oct 14, 2024

Could you fix CI/clippy errors?

The clippy warning is not related to this change. Just a new warning on nightly.

https://github.com/rust-embedded/cargo-binutils/pull/148/files

@PhilippMolitor
Copy link
Contributor Author

Correct, I think none of the failing CI scripts have to do with my changes. I built and installed this on my machine on latest rust stable without any issues.

One more comment because i noticed it with my current work that uses cargo-binutils:
Some of the flags that are re-implemented from cargo, like --release and --no-default-features are not implemented correctly, they still require a value instead of just working as a flag.
With clap, the Arg builder needs this additional setting: .action(ArgAction::SetTrue), and it also should be retrieved as a flag instead of a normal .get_one() parameter.
The current workaround is to pass --some-cargo-flag true (or any other value).

@Emilgardis Emilgardis enabled auto-merge October 16, 2024 16:33
@Emilgardis Emilgardis disabled auto-merge October 16, 2024 16:34
@Emilgardis
Copy link
Member

can you please remove the merge commit? same goes for the other prs

@burrbull burrbull added this pull request to the merge queue Oct 16, 2024
Merged via the queue into rust-embedded:master with commit e8c5027 Oct 16, 2024
8 checks passed
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.

4 participants