cargo fmt + GH workflow

[willcl-ark]

If cargo fmt-ed code is desirable here, the first commit runs it initially, and the second provides a GH action to check cargo fmt, cargo doc and cargo clippy, which was extracted and modified from https://github.com/jonhoo/faktory-rs

Example failure and pass of the actions here

[achow101]

cargo clippy outputs some warnings for me but they don't show in ran action?

warning: direct implementation of `ToString`
   --> src/main.rs:94:1
    |
94  | / impl ToString for NodeAddress {
95  | |     fn to_string(&self) -> String {
96  | |         match &self.host {
97  | |             Host::Ipv4(ip) => format!("{}:{}", ip, self.port),
...   |
101 | |     }
102 | | }
    | |_^
    |
    = help: prefer implementing `Display` instead
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#to_string_trait_impl
    = note: `#[warn(clippy::to_string_trait_impl)]` on by default

warning: writing `&String` instead of `&str` involves a new object where a slice will do
   --> src/main.rs:958:16
    |
958 |     seed_name: &String,
    |                ^^^^^^^ help: change this to: `&str`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
    = note: `#[warn(clippy::ptr_arg)]` on by default

warning: single-character string constant used as pattern
    --> src/main.rs:1044:50
     |
1044 |                     if !filter_label.starts_with("x") || filter_label.starts_with("x0") {
     |                                                  ^^^ help: consider using a `char`: `'x'`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
     = note: `#[warn(clippy::single_char_pattern)]` on by default

[willcl-ark]

Yes, this is somewhat on purpose. I thought it could be a bit annoying to have all clippy lints fail CI, but this is possible to enable by using -Dwarnings flag with clippy.

In addition, this will only check diff for failures (which can also be configured), but I think that's more sensible?

Finally, there was also a bug -- reviewdog was trying to use "github checks" APi (whatever that is exactly), which requires a GitHub App or something to run. It's apparently "better", but sounds more annoying to me.

So currently I have it commenting >= warnings in my fork: https://github.com/willcl-ark/dnsseedrs/pull/1 but it still "passes" CI. I think this is a good combination, but can have it fail on warnings if perferred?

[achow101]

I think having it fail on warnings is fine. I don't know enough rust to be opinionated, and if there are linters that are, then that's all the better. Also this project isn't big enough to care about only checking diffs yet, I'm fine with formatting and fixing style-like errors in one go now, and then having the linters yell at me for future changes.

[willcl-ark]

Ok done. Also, that reviewdog was a pain, so reverted to regular CI failures (i.e. check the logs) and simpler CI workflow file.

Example of failure without the included clippy commit here: https://github.com/willcl-ark/dnsseedrs/actions/runs/8411930712?pr=1

[0xB10C]

fwiw this doesn't check that the code actually builds

you'd need a cargo build step.

[willcl-ark]

fwiw this doesn't check that the code actually builds

you'd need a cargo build

This was intentional -- it's just supposed to format/lint/doc. I usually see the build implicit to a cargo test job in CI, but as there are no tests here, perhaps it would be wise to add a build step...

[achow101]

I usually see the build implicit to a cargo test job in CI,

I'll need to find another free week to learn to write tests in rust.