From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 09 Oct 2025 05:29:42 GMT
Subject: Re: [PATCH 2/6] ci: check formatting of our Rust code
Message-ID: <aOdIRnB-SGQwj935@pks.im>
In-Reply-To: <aObPzzLtZzodZf+Q@szeder.dev>

On Wed, Oct 08, 2025 at 10:55:43PM +0200, SZEDER Gábor wrote:
> On Tue, Oct 07, 2025 at 02:36:30PM +0200, Patrick Steinhardt wrote:
> > Introduce a CI check that verifies that our Rust code is well-formatted.
> > This check uses rustfmt(1), which is the de-facto standard in the Rust
> > world.
> > 
> > The rustfmt(1) tool allows to tweak the final format in theory. In
> > practice though, the Rust ecosystem has aligned on style "editions".
> > These editions only exist to ensure that any potential changes to the
> > style don't cause reformats to existing code bases. Other than that,
> > most Rust projects out there accept this default style of a specific
> > edition.
> > 
> > Let's do the same and use that default style. It may not be anyone's
> > favorite, but it is consistent and by making it part of our CI we also
> > enforce it right from the start.
> > 
> > Note that we don't have to pick a specific style edition here, as the
> > edition is automatically derived from the edition we have specified in
> > our "Cargo.toml" file.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> 
> > diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh
> > new file mode 100755
> > index 0000000000..082eb52f11
> > --- /dev/null
> > +++ b/ci/run-rust-checks.sh
> > @@ -0,0 +1,12 @@
> > +#!/bin/sh
> > +
> > +. ${0%/*}/lib.sh
> > +
> > +set +x
> > +
> > +if ! group "Check Rust formatting" cargo fmt --all --check
> > +then
> > +	RET=1
> > +fi
> > +
> > +exit $RET
> 
> Our ci/*.sh scripts usually rely on 'set -e' to catch failed commands.
> Either this script should follow that convention as well, or the
> commit message should justify the deviation from convention.

Ah, good point. The reason is that subsequent commits add more checks,
and I want to make sure that they all run even if previous checks
failed. It's otherwise annoying to fix a first set of errors surfaced by
the CI only to then notice that later checks also fail.

I'll mention this in the commit message.

Patrick