testing/mozbase/rust/mozdevice/src/lib.rs: Fix the clippy warnings
Categories
(Testing :: Mozbase Rust, task, P5)
Tracking
(firefox88 fixed)
| Tracking | Status | |
|---|---|---|
| firefox88 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: me, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug, Whiteboard: [lang=rust])
Attachments
(1 file, 1 obsolete file)
+++ This bug was initially created as a clone of Bug #1622690 +++
Filling as a good first bug to learn workflows.
As the change is easy, it is mostly to learn how to contribute to Firefox.
To run the linter:
$ ./mach lint -l clippy testing/mozbase/rust/mozdevice/src/lib.rs --warnings
Warnings:
xxx/testing/mozbase/rust/mozdevice/src/lib.rs
29:39 warning trivial regex clippy::trivial_regex (clippy)
88:44 warning redundant clone clippy::redundant_clone (clippy)
278:25 warning calling `to_string` on `&&std::string::String` clippy::inefficient_to_string (clippy)
Tutorial to contribute:
https://firefox-source-docs.mozilla.org/tools/docs/contribute/how_to_contribute_firefox.html
Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.
|
Assignee | |
Comment 1•4 years ago
|
||
Fix simple linting errors
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
||
Comment 2•4 years ago
|
||
This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.
|
||
Comment 4•4 years ago
|
||
Well it was, but not sure if it still applies. I might expect merge issues.
Given that Sylvetre mentores this bug, I will needinfo him for follow-up, and also move the bug to the correct component.
| Comment hidden (duplicate) |
|
Reporter | |
Comment 6•4 years ago
|
||
I just run it and it is still valid:
19:02:05.933 clippy (810969) | Finished in 435.81 seconds
/home/sylvestre/dev/mozilla/mozilla-unified/testing/mozbase/rust/mozdevice/src/lib.rs
33:39 warning trivial regex clippy::trivial_regex (clippy)
97:18 error using `to_string` in `fmt::Display` implementation might lead to infinite recursion clippy::to_string_in_display (clippy)
|
|
||
Comment 7•4 years ago
|
||
Ok, great. So when the patch still applies can you land it?
|
|
Reporter | |
Comment 8•4 years ago
|
||
it doesn't apply and not sure why you reassigned to Darrien as the bug didn't move for 5 months
|
|
||
Comment 9•4 years ago
|
||
The patch was ready to land by that time, but no-one actually landed it. It was outside of our component and as such it wasn't clear that Darrien doesn't have permissions.
Darrien, would you be interested to update the patch? I would appreciate.
|
|
Assignee | |
Comment 10•4 years ago
|
||
Hey! I don't mind updating the patch. You'll have to give me a few days, I haven't worked on Firefox in a bit. But will get to it shortly!
|
|
||
Comment 11•4 years ago
|
||
Thanks a lot, and yes take your time.
|
|
Assignee | |
Comment 12•4 years ago
|
||
|
|
Assignee | |
Comment 13•4 years ago
|
||
Hey folks, I have a new patch up that fixes all of the new clippy errors: https://phabricator.services.mozilla.com/D105539
I'm a little sketchy on the commands to update the previous commit so I made a new patch. That also means the old one can be removed.
|
|
||
Updated•4 years ago
|
|
||
Comment 14•4 years ago
|
||
|
||
Comment 15•4 years ago
|
||
Backed out for causing bustages.
Backout link: https://hg.mozilla.org/integration/autoland/rev/c21bf4c05a8669930ea86b921a101526e098835d
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-rusttests%2Fdebug%2Cbr&revision=e1972a4c557a58a1294138829b6290cd09dbce84
Failure log: https://treeherder.mozilla.org/logviewer?job_id=333393689&repo=autoland&lineNumber=8324-8325
|
|
||
Comment 16•4 years ago
|
||
There is a unit test failure:
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - test shell::tests::escape_newline ... FAILED
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - failures:
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - ---- shell::tests::escape_newline stdout ----
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - thread 'shell::tests::escape_newline' panicked at 'assertion failed: `(left == right)`
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - left: `"\\\\n"`,
[task 2021-03-16T14:44:27.335Z] 14:44:27 INFO - right: `"\\\'\n\'"`', testing/mozbase/rust/mozdevice/src/shell.rs:64:9
There might be something wrong in the logic of the code given that the final replacement for '\n' will never be run. At this time \n has already been converted to \\\\n, so that the pattern doesn't match. But that's not a regression from fixing the clippy warnings, and should be investigated separately.
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Comment 17•4 years ago
|
||
I updated the patch to fix the test failure, and triggered its landing.
|
|
||
Comment 18•4 years ago
|
||
|
||
Comment 19•4 years ago
|
||
| bugherder | ||
|
|
||
Comment 20•4 years ago
|
||
Darrien, I want to thank again for getting this bug fixed! By surprise it even fixed a stack overflow crash. As such we might have to always take care of clippy warnings in the future.
|
|
Assignee | |
Comment 21•4 years ago
|
||
I'm just glad I was able to help fix something with my favorite browser. Thanks for helping this land!
|
|
Reporter | |
Comment 22•4 years ago
|
||
Thank YOU
I didn't realize that this would fix a crash itself :)
|
|
||
Comment 23•4 years ago
|
||
Feel free to reach out in case you are interested to work on something else. If yes and it would be Rust, JS, or Python related, and in combination with automation feel free to join us in https://chat.mozilla.org/#/room/#interop:mozilla.org.
|
|
Assignee | |
Comment 24•4 years ago
|
||
I appreciate the guidance. I'm in the process of buying a house right now and am a little busy, but will certainly join afterwards. Thanks again :)
Description
•