kindelia / Kindelia

An efficient, secure cryptocomputer
https://kindelia.org/
602 stars 41 forks source link

Can we please have CI check for rustfmt? #270

Open dan-da opened 1 year ago

dan-da commented 1 year ago

Presently we have a check for clippy errors, but badly formatted code is allowed.

Once some pending PRs have been reviewed+merged, I would be happy to make a PR that runs rustfmt over the entire repo, at which point we could enable a CI check and enforce rustfmt style going forward....

racs4 commented 1 year ago

This was already proposed, but was not considered because some people who already participated more actively in the development of the project did not like the style proposed by cargo fmt.

This is summed to other aspects such as word alignment, which with formatting would be lost, such as

let sum  = 3   + 2;
let sum1 = sum + 2;
let sum2 = sum + 4;

which would lose that "pretty style" if formatted (most code like that was found in the hvm.rs file, which contained the Runtime, Term, Statements, etc; later this file was split, but I believe there is still code like that in the kindelia_core/runtime/mod.rs file and others resulting from this division). Some people used and defended this style, and because of this "discussion" cargo fmt was not used in CI.

dan-da commented 1 year ago

Ok, thx for sharing that history! I don't wish to step on any toes, but I will make some counterpoints for sake of discussion and moving ball foward, hopefully constructively.

aspects such as word alignment

Formatting can be preserved within a code block (statement, function, module) using eg #[rustfmt::skip] and files can be skipped with rustfmt.toml.

I understand that skipping at block level is not perfect when one really just wants to skip the next 5 lines or whatever. But the ability to just skip a fn should probably suffice?

Also rustfmt default settings can be overridden with rustfmt.toml which gives some control over rustfmt's style.

still code like that in the kindelia_core/runtime/mod.rs file and others

yeah, so those files could be ignored by rustfmt entirely as a first step.


I totally get the desire to preserve space alignment or other custom formatting for some code that looks better that way.

At the same time, by not using rustfmt, we are losing the benefits of standardized formatting for all the rest (large majority I think) of the code that does not require such.

So I would advocate that we reverse the situation and rustfmt the majority of code and skip the minority that needs custom formatting.

I think this will make kindelia code more accessible for others in the rust ecosystem. And as more contributors submit PRs it is best practice to have a standard coding style, and rustfmt makes that very easy to achieve and enforce.

and what is the alternative? That each contributor submits PRs in whatever style suits them at the time for the entire codebase without fixing up newlines, extra spaces, etc? That seems rather messy...