CI Format check

[jbesraa]

Is there any reason we check formatting in nightly and not stable only?

IMO it would be simpler to work on the project when its just stable as usually text editors are configured against stable rustup as a default

[DanGould]

stable toolchain is missing many lints we use nightly for

https://github.com/rust-bitcoin/rust-bitcoin/issues/2128

this was solved there with an automated ci formatter that runs weekly https://github.com/rust-bitcoin/rust-bitcoin/pull/2135

[jbesraa]

A weekly formatter gh job sounds good to me, but that will introduce a different problem with PRs that can have some linting changes unrelated to PR and that can make it tricky to review.

I would suggest to force stable in PR checks and we can actually trigger a github action to nightly format before each release(so that might change release flow adding the formatting part)

[DanGould]

One way I'm thinking of fixing this is by including a reproducible direnv devShell from a nix.flake that includes the right version of cargo. It still might not please everyone but it'll make it easy to reproduce the environment with the right tools to format. I'd rather each commit is formatted than extra regular linting commits.

[jbesraa]

TBH iam still not fully convinced its necessary to use nightly.

• The post you linked from rust-bitcoin is from one year ago, things probably changed since then. I think you should write down which lint/format rules you are interested in and we can take it from there.

• Nightly features are not set in stone and can change or get removed before going stable, so that can also result in lint commits. That is also correct for stable, but can happen less often.

• Nightly rust doesnt work great in some text editors and usually people use "stable" as their formatter. in our project that can really cause a problem(re https://github.com/payjoin/rust-payjoin/pull/169/files) for new comers. it also require the developer to switch to nightly format the code and then start commiting.

[DanGould]

There are also separate formatting jobs right now for payjoin-cli and payjoin which ignore payjoin-directory. It would make sense to do what rust-lightning does: Have the job format everything except excluded files.

After specifying policy in the readme perhaps we can provide a nix flake with the necessary environment to format, build & test code. A CI job could also periodically fmt but I'd prefer we keep a clean history by just setting formatting rules as a requirement to merge. I'm not sure how installing nightly rust is any different from installing stable rust, but if stable contains all of the fmt rules I'm fine using that instead.

I feel like rust-bitcoin and rust-lightning do way more reasoned bikeshedding on formatting than I'd ever like to and think simply adapting their practices to this repository is the most effective strategy.

edit: rust-lightning uses stable formatter, not nightly.

[DanGould]

Closing this since #297 in favor of requiring formating at the time of PR for now.