Configure CI including clippy #27

tcharding · 2022-06-30T05:38:02Z

Do some non-controversial clippy fixes. PR is now just the patches that were a firm ack below.

stevenroose · 2023-02-23T03:20:25Z

2d5d328 add clippy configuration file: ack, interesting Clippy has a msrv field
6d94c16 Remove unneeded 'static lifetime: ack
968329f Use struct initialisation shorthand: not a fan of these shorthands, it convolutes field names and local variable names
e9e33ea Make operator precedence explicit: ack
cc8bbaa Use byte literal instead of cast: ack
4ec4d11 Use iter_mut().enumerate() instead of array access: ack I guess, but that one with take() isn't that clear IMO and it's in a test anyway..
fa2b19d Remove unneeded references: ack
366e410 Remove explicit lifetime: ack
44f7f0e Use contains combinatior: eh, you can't write a simple x < Y | x > Z anymore in Rust?..
1a5b4cf Remove unneeded return statement: ack
f1c4c7d Use is_empty: ack
0c8e968 Remove useless question mark operator: these two changes are not entirely identical, I don't see how clippy things they are better.. other than having 5 less characters, but they are less flexible semantically..
0fce400 Remove useless use of vec macro: ack
9036eee Use unwrap_or_else with panic in test code: I don't think this is better..
b233d33 Remove unused lifetime: ack
077721c Feature gate generate_in call: ack, this is more a bug than a clippy suggestion I guess..
148025a Use hyperlink: ack
1c70579 Fix build for serde with no default features: this is also not clippy, right? this seems like a bug too.. ack
dc19280 Add a test script: ack as I guess this only adds the script and doesn't use it yet?
4882055 (origin/pr/27) Add CI by way of GitHub actions: I don't know github actions too well, but it seems like this enforces zero clippy warnings, which doesn't make sense to me.. clippy gives suggestions not problems

tcharding · 2023-02-23T03:38:37Z

Good to see you bro! Will re-spin with just the acks.

tcharding · 2023-02-23T22:25:03Z

I don't know github actions too well, but it seems like this enforces zero clippy warnings, which doesn't make sense to me.. clippy gives suggestions not problems

Yes it does. How we solve this will effect how I proceed with this PR. If we don't want to enforce clippy in CI then I can move all the clippy fixes to a separate PR and just do an "add github actions" PR.

I've done #42 to mimize the scrope, the clippy stuff can come later.

stevenroose · 2023-02-28T04:51:53Z

clippy.toml

@@ -0,0 +1 @@
+msrv = "1.41.1"


I'm interested what this does exactly.

stevenroose · 2023-02-28T04:53:15Z

utACK 38afbed

It's rebased on the github ci changes right? So I guess if you unmark it as draft, it will run CI and I can merge it :) Thanks!

tcharding · 2023-02-28T21:34:13Z

Ah I did the original work with clippy configured for 1.41.1 so I can't guarantee that all the changes are sound and correct for Rust 1.29 and I was too lazy to go back over them all, thought I'd just wait till we moved the MSRV since its in the pipeline already.

Add a `clippy.toml` configuration file using the current MSRV of 1.41.1

Clippy emits: warning: constants have by default a `'static` lifetime We have edition 2018 now so we do not need the explicit static lifetime.

clippy emits: warning: operator precedence can trip the unwary As suggested, add parenthesis to make the operator precedence explicit.

Clippy emits: warning: casting a character literal to `u8` truncates As suggested, use byte literal instead of a cast.

Clippy emits a few warnings of form: warning: this expression creates a reference which is immediately dereferenced by the compiler Remove unneeded references.

Clippy emits: warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) As suggested, remove explicit lifetime.

Clippy emits: warning: unneeded `return` statement As suggested, remove the unneeded return statement.

Clippy emits warning: length comparison to zero As suggested, use `is_empty`.

Clippy emits: warning: useless use of `vec!` As suggested, remove the useless call to `vec!` and use the slice directly.

Clippy emits: warning: this lifetime isn't used in the impl Remove unused lifetime.

Building the docs throws a warning: warning: this URL is not a hyperlink As suggested, use a hyperlink.

tcharding · 2023-07-12T02:57:25Z

Do you want this @stevenroose? If not please close - I'm not fussed either way. Thanks

stevenroose · 2023-08-28T01:40:29Z

If you rebase it, I think I'd ack it :) I'd have to re-review.

tcharding · 2023-08-28T03:30:22Z

No worries, I'll come back to it.

notmandatory mentioned this pull request Feb 23, 2023

Add rust ci workflow #33

Closed

tcharding force-pushed the 06-30-clippy branch 2 times, most recently from f80aa90 to 38afbed Compare February 28, 2023 00:13

tcharding marked this pull request as draft February 28, 2023 00:14

tcharding mentioned this pull request Feb 28, 2023

Various improvements #39

Closed

stevenroose reviewed Feb 28, 2023

View reviewed changes

clippy.toml

@@ -0,0 +1 @@

msrv = "1.41.1"

Copy link

Collaborator

stevenroose Feb 28, 2023

Choose a reason for hiding this comment

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

I'm interested what this does exactly.

tcharding added 11 commits March 1, 2023 09:51

add clippy configuration file

cbff33a

Add a `clippy.toml` configuration file using the current MSRV of 1.41.1

Remove unneeded 'static lifetime

07c3e50

Clippy emits: warning: constants have by default a `'static` lifetime We have edition 2018 now so we do not need the explicit static lifetime.

Make operator precedence explicit

4d7e783

clippy emits: warning: operator precedence can trip the unwary As suggested, add parenthesis to make the operator precedence explicit.

Use byte literal instead of cast

1f05388

Clippy emits: warning: casting a character literal to `u8` truncates As suggested, use byte literal instead of a cast.

Remove unneeded references

a8bfa8b

Clippy emits a few warnings of form: warning: this expression creates a reference which is immediately dereferenced by the compiler Remove unneeded references.

Remove explicit lifetime

60afec7

Clippy emits: warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) As suggested, remove explicit lifetime.

Remove unneeded return statement

c246aac

Clippy emits: warning: unneeded `return` statement As suggested, remove the unneeded return statement.

Use is_empty

0b58580

Clippy emits warning: length comparison to zero As suggested, use `is_empty`.

Remove useless use of vec macro

ca35785

Clippy emits: warning: useless use of `vec!` As suggested, remove the useless call to `vec!` and use the slice directly.

Remove unused lifetime

41eddb9

Clippy emits: warning: this lifetime isn't used in the impl Remove unused lifetime.

Use hyperlink

a083faf

Building the docs throws a warning: warning: this URL is not a hyperlink As suggested, use a hyperlink.

tcharding force-pushed the 06-30-clippy branch from 38afbed to a083faf Compare February 28, 2023 22:51

tcharding marked this pull request as ready for review February 28, 2023 22:51

tcharding closed this Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure CI including clippy #27

Configure CI including clippy #27

tcharding commented Jun 30, 2022 •

edited

Loading

stevenroose commented Feb 23, 2023

tcharding commented Feb 23, 2023

tcharding commented Feb 23, 2023

stevenroose Feb 28, 2023

stevenroose commented Feb 28, 2023

tcharding commented Feb 28, 2023

tcharding commented Jul 12, 2023

stevenroose commented Aug 28, 2023

tcharding commented Aug 28, 2023

Configure CI including clippy #27

Configure CI including clippy #27

Conversation

tcharding commented Jun 30, 2022 • edited Loading

stevenroose commented Feb 23, 2023

tcharding commented Feb 23, 2023

tcharding commented Feb 23, 2023

stevenroose Feb 28, 2023

Choose a reason for hiding this comment

stevenroose commented Feb 28, 2023

tcharding commented Feb 28, 2023

tcharding commented Jul 12, 2023

stevenroose commented Aug 28, 2023

tcharding commented Aug 28, 2023

tcharding commented Jun 30, 2022 •

edited

Loading