script: Add features and rustdoc checks

[febo]

Problem

Currently there is no automated way to check if every combination of features is working properly on rust crates (interface or client) nor if the code on rustdoc compiles.

Solution

This PR adds two script to perform additional steps:

• check-features: uses cargo-hack to perform a feature powerset check.
• rustdoc: runs rustdoc build the documentation.

These scripts run by default on CI and can also be run locally.

Thanks @kevinheavey for suggesting adding these!

[lorisleiva]

I think this is fine but just curious why we can't just stick these checks inside the lint script?

[febo]

We could, I was doing that first. The only caveat is that if we want to pass specific command-line arguments, it won't be easy. Should I add both steps to the lint script?

[lorisleiva]

I'm not sure how often these scripts are used in isolation. From an outsider perspective, I kinda just want a script or two that tells me if my code is alright plus one that test my code. That's why you added the new "check" script. Because the amount of scripts is becoming overwhelming. I think "categorising" these within the broader concept of "linting" the code makes sense for me but again, I'm not an experienced Rustacian so I'm not sure this is what people will expect.

[febo]

Yeah, exactly, that is the reason that I added a check that runs all of them since that is probably what you want locally. I left them separate for the CI so when it fails, it is more obvious which one has a problem.

[febo]

But I am not sure, I think we need @joncinque and @buffalojoec opinion as well. ☺️

[joncinque]

There are quite a few scripts, with a lot of copy-pasta between them. It might be nice to only have language-specific scripts, ie. scripts/rust and scripts/js that can be reused for all rust packages, or all js packages, etc.

Personally, I've found it slightly annoying to update all the scripts when making a change, but maybe it doesn't happen too often.

[buffalojoec]

I mostly agree on aggregate language-specific scripts, but I think it's probably easier to do one script per language, per library (ie. scripts/program, scripts/client/rust, scripts/client/js). Mostly because the program will use -sbf related commands and configs. Not a huge deal either way, though.

I don't love the fact that the number of scripts is growing, but it does make sense to me for clippy and check (hack) to be separate jobs, invokable separately. Including it in the format_and_lint CI job is totally fine, though.

[febo]

Yeah, having many similar scripts is not ideal. I had a chat with Loris and we came up with the following. Divide the scripts by language, as Jon suggested (script/js and scripts/rust), and have a "configuration" script specific for each package when needed.

The scripts directory would look like:

scripts
   |
   +- js
   |  |
   |  +- format.mjs
   |  +- lint.mjs
   |  +- publish.mjs
   |  +- test.mjs
   |
   +- rust
   |  |
   |  +- build.mjs
   |  +- clean.mjs
   |  +- dump.mjs
   |  +- format.mjs
   |  +- lint-clippy.mjs
   |  +- lint-docs.mjs
   |  +- lint-features.mjs
   |  +- lint.mjs
   |  +- test.mjs
   |  
   +- crate
      |
      +- client.mjs
      +- interface.mjs
      +- program.mjs

All rust scripts then accept a "crate" parameter, which specify the custom "configuration" script to use (folder crate). In these scripts we can add any specific variables for the crate.

In summary, we will have generic language scripts + specific configuration ones. Since this is a bigger refactoring, the idea is to do this in a separate PR.

[joncinque]

Looks good to me (with Loris's comments addressed)

[joncinque]

There are quite a few scripts, with a lot of copy-pasta between them. It might be nice to only have language-specific scripts, ie. scripts/rust and scripts/js that can be reused for all rust packages, or all js packages, etc.

Personally, I've found it slightly annoying to update all the scripts when making a change, but maybe it doesn't happen too often.

[buffalojoec]

I'm not against an aggregate pnpm script that runs format, lint, and test all in one go.

[buffalojoec]

I mostly agree on aggregate language-specific scripts, but I think it's probably easier to do one script per language, per library (ie. scripts/program, scripts/client/rust, scripts/client/js). Mostly because the program will use -sbf related commands and configs. Not a huge deal either way, though.

I don't love the fact that the number of scripts is growing, but it does make sense to me for clippy and check (hack) to be separate jobs, invokable separately. Including it in the format_and_lint CI job is totally fine, though.

[buffalojoec]

Wfm, as long as we're planning to move toward grouping the scripts by language, as mentioned in discussions. Lots of scripts!

[buffalojoec]

nit: lint-rust-all.mjs?

[lorisleiva]

I have a slight preference over lint than lint-all because it makes it clearer that lint is a parent of lint-*. Not massively fussed though.

[buffalojoec]

I also don't feel strongly, but the -all for me is usually indicative that it includes multiple combined steps.

[lorisleiva]

🤷‍♂️😅

[febo]

lint keeps it "symmetric" with the JS ones. In any case, lint-rust will go away when we restructure the scripts, so perhaps we can remove the lint that call all of them under the same script on the package.json instead.

[buffalojoec]

Ship it!

[joncinque]

Just a couple of little nits, but this should be ready!