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

clippy: Fix several warnings in components/devtools #31501

Merged
merged 1 commit into from
Mar 5, 2024
Merged

Conversation

eerii
Copy link
Contributor

@eerii eerii commented Mar 4, 2024

Fixes several clippy warnings in the components/devtools directory. It is part of #31500.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes are part of Fix as many clippy problems as possible #31500.
  • These changes do not require tests because they do not modify functionality, they only fix lint errors.

@mrobinson
Copy link
Member

@eerii Nice work. This change looks excellent. One thing you might need to do is to run ./mach fmt before uploading it.

@eerii
Copy link
Contributor Author

eerii commented Mar 5, 2024

@eerii Nice work. This change looks excellent. One thing you might need to do is to run ./mach fmt before uploading it.

Thanks! I forgot about it last time. I have a couple of doubts

  • Now that there was a redundant match removed from components/devtools/actors/emulation.rs, msg_type is unused. Should I remove it and fix the function invocations or leave it and just mark it with an underscore?
  • For clippy warnings about functions with too many arguments, should I add #[allow(clippy::too_many_arguments)] or leave the warning?

@mrobinson
Copy link
Member

* Now that there was a redundant match removed from `components/devtools/actors/emulation.rs`, `msg_type` is unused. Should I remove it and fix the function invocations or leave it and just mark it with an underscore?

I think you should leave the argument as that function is implementing a trait. Simply prefix it with an underscore like the other unused arguments.

* For clippy warnings about functions with too many arguments, should I add `#[allow(clippy::too_many_arguments)]` or leave the warning?

Yes, I think that's a reasonable approach.

Thank you!

@eerii
Copy link
Contributor Author

eerii commented Mar 5, 2024

Thank you! I will do that and make sure everything works, and then I'll update the pr

@eerii eerii changed the title clippy: Fix several warnings in component/devtools clippy: Fix several warnings in components/devtools Mar 5, 2024
@eerii eerii marked this pull request as ready for review March 5, 2024 09:22
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Nice work!

@mrobinson mrobinson enabled auto-merge March 5, 2024 15:15
@mrobinson mrobinson added this pull request to the merge queue Mar 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 5, 2024
@mrobinson mrobinson added this pull request to the merge queue Mar 5, 2024
Merged via the queue into servo:main with commit 3552bb2 Mar 5, 2024
9 checks passed
@eerii eerii mentioned this pull request Mar 5, 2024
4 tasks
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.

None yet

2 participants