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

rust: avoid warnings from rust.test #13919

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

bonzini
Copy link
Contributor

@bonzini bonzini commented Nov 18, 2024

Any argument from the base target is copied to the test target, but some keyword arguments for libraries are not available in executable. Remove them, and add a unit test that checks for it with --fatal-meson-warnings.

@bonzini bonzini requested a review from jpakkane as a code owner November 18, 2024 11:20
Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

One small nit to use a list instead of a tuple, but otherwise this looks good.

mesonbuild/modules/rust.py Outdated Show resolved Hide resolved
@dcbaker
Copy link
Member

dcbaker commented Nov 18, 2024

Thanks, looks good to me!

@eli-schwartz
Copy link
Member

Second commit should be a --fixup and squashed, surely?

@bonzini
Copy link
Contributor Author

bonzini commented Nov 18, 2024

Second commit should be a --fixup and squashed, surely?

To be honest, since Dylan used the suggested changes functionality I just did whatever GitHub told me to do. I can push the squashed version tomorrow.

@eli-schwartz
Copy link
Member

The "suggested changes" functionality is for suggesting changes, not committing changes. :) Github doesn't allow you to do one without the other, and it's a wretched mess because github is chronically incapable of enabling correct git workflows when using the web UI to synthesize git commits.

Github staff apparently do not believe that there is any valid way to merge a PR other than by "squash all commits into one using a 5,000-line generated commit message consisting of the --oneline description of each commit including 'address feedback' and 'fixup' and 'revert previous attempt' and 'lint'".

@dcbaker
Copy link
Member

dcbaker commented Nov 18, 2024

Yeah, I always take the "suggested commits", commit them, pull them locally, and then just squash them before repushing. I find I get better results with that approach than manually retyping locally.

Any argument from the base target is copied to the test target, but some
keyword arguments for libraries are not available in executable.
Remove them.

Signed-off-by: Paolo Bonzini <[email protected]>
@dcbaker dcbaker merged commit 110e2de into mesonbuild:master Nov 19, 2024
30 of 33 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.

3 participants