Run clippy-check action on CI

[silvanshade]

This is a follow up draft PR for #477 that runs clippy on CI. It is not using actions-rs/clippy-check yet but I will switch over to that shortly.

I've created this as a draft because it looks like there are some additional clippy warnings that will need to be fixed first.

I've also just replaced the check step with clippy since running both would be a little redundant (cargo clippy runs cargo check already). However, I can change this if you'd like to keep them separate.

[silvanshade]

I think this is ready for review. Note that the CI will fail until the token is setup. I couldn't find which permissions are needed for the token from clippy-check README but I think it's probably just repo.

[hawkw]

I've also just replaced the check step with clippy since running both would be a little redundant (cargo clippy runs cargo check already). However, I can change this if you'd like to keep them separate.

I'd prefer to run clippy separately. The intention behind the check step was for the build to fail fast if the code doesn't compile at all, so that other steps don't need to be run. I would prefer to run clippy in the "warnings" step, instead, which I notice this branch removes.

[silvanshade]

Sounds good, I'll make the suggested changes shortly.

[silvanshade]

@hawkw Okay I've made the suggested changes!

[hawkw]

Besides fixing the build, I think this looks good to me. I'll figure out the token env var before merging & let you know if there are any other problems.

[silvanshade]

@hawkw Okay, I think it's ready now. After I fixed the issue you pointed out with extern crate, several more little errors popped up 😅 But it looks like the tests are passing now 🙂

[silvanshade]

Just FYI, I asked about the token on the actions-rs gitter and this is what svartalf said:

GITHUB_TOKEN should be used preferably, but if you was to apply another token, you will need the r/w access to the Checks API

[silvanshade]

Something I didn't realize about GITHUB_TOKEN is that it's auto-generated. So there's no need to actually create a separate token like I was suggesting. I've updated the PR to use that, so everything should just work without further configuration now.

[hawkw]

@darinmorrison can I get you to rebase the latest master? Hopefully this will then build successfully and we can get it merged!

Sorry for keeping you waiting, I was on vacation.

[silvanshade]

@hawkw no problem. Hope you had a nice vacation!

I went ahead and rebased this. It's failing on the features-nightly test though. It says #![feature] may not be used on the stable release channel which is odd because as far as I can tell, that job should be using nightly.

Maybe it's actually complaining because the #![feature(async_await)] isn't needed anymore? Should I remove that or does it not matter?

[hawkw]

Yeah, the features-nightly test is supposed to run on a nightly compiler. However, all the nightly features we use are now stable, so that test needs to eventually be run as a normal test instead. For now I think it's fine to remove the #![feature(async_await)] so we can merge this PR.

[silvanshade]

@hawkw I removed that #![feature(async_await)] and the test that was failing is passing now. Should be good to go now I think.

[hawkw]

looks good to me