Add rustfmt and clippy to the CI

[mariocynicys]

This PR adds a new job named lint that runs cargo fmt on the project and then cargo clippy. This won't apply fixes, but will only report them and fail the workflow if there were any.

Further improvements:

• We can have the build job depending on lint.

[sr-gi]

Nice, I wanted to add this when I added the CI, but there were some clippy issues popping up with atomic types, looks like those has been finally fixed.

I'm wondering if we want to make the build fail due to clippy or just warn so the reviewers can give a look at what may need improvement. I'm mainly concerned about issues like the one I was mentioning before (where some lints were being reported but they were actually changing the functionality) that may block us from merging. We can always force merge if those pop up, but it's kind of a bad practice I think.

[mariocynicys]

@sr-gi That's why I opted out of making the build job depend on lint for the mean time. I also faced the atomic warning issue in my local environment.

I think we can force merge at any time issues popup. The main reason I added the --deny warnings flag is to have it clear form outside (with the red :x: mark) that the lint either failed or raised a warning, without contributors having to manually check the the action's logs for warnings with every push. Also typically, there won't be a lot of false positives (and we can suppress them from inside the code with #[allow(clippy::bad_rule)] or in clippy.toml file).

I will see if we can change the Successful status color from inside the workflow conditionally. Like maybe a yellow or white tick ✔️ to indicate that there are warnings but no failures.

[mariocynicys]

I have changed it so that a warning annotation is added when clippy warnings are found. This won't fail the workflow on warnings but one should then check the workflow summary for any warnings before merging the PR.

Edit: actions-rs/clippy-check already provides a better functionality, so using it instead. clippy-check needs a github secret token to be able to write annotations.

[sr-gi]

I have changed it so that a warning annotation is added when clippy warnings are found. This won't fail the workflow on warnings but one should then check the workflow summary for any warnings before merging the PR.

This shouldn't be much of an issue given not many will have merge privileges. I normally check all that locally, so it'll be already an improvement. Is there any way that that warning is marked somehow? Something in between ❌ and ✅ would be ideal to notice that.

Edit: actions-rs/clippy-check already provides a better functionality, so using it instead. clippy-check needs a github secret token to be able to write annotations.

What I've noticed is that there's a limitation regarding running this from a forked repo, so looks like it's not much of a use (the ability to have write access I mean) given all PRs are authored from forked repos?

Source: https://github.com/actions-rs/clippy-check#limitations

[mariocynicys]

Is there any way that that warning is marked somehow? Something in between ❌ and ✔️ would be ideal to notice that.

Currently, no, there is no warning mark, but it was requested here.

What I've noticed is that there's a limitation regarding running this from a forked repo, so looks like it's not much of a use (the ability to have write access I mean) given all PRs are authored from forked repos?

I apparently misunderstood the limitations section of clippy-check. You are right, it won't be so useful that way. We can use the solution in the commit (which causes an annotation with a warning mark to be displayed in the workflow summary ) or fail the workflow to indicate warnings (I prefer failing since its visual impact is greater). Any other ideas?

[sr-gi]

I think any of those two is fine. Whatever'd rather.

If you fancy the "fail route" better, we could go that way until the warning feature is implemented (if ever). It should be fine as long as there are no broken things with clippy.

PS: That means we'll need to annotate some of the current stuff, such as the UUID struct.

[sr-gi]

e64a934118c9febbba34fd9464cd1b569301228c can be squashed into 24a5a2ab3f5954025d9c76451feb350b8bf46813. Apart from that, LGTM.