Re: [PATCH v2 3/5] ci: separate whitespace check script

> On Thu, May 02, 2024 at 02:38:37PM -0500, Justin Tobler wrote:
> > The `check-whitespace` CI job is only available as a GitHub action. To
> > help enable this job with other CI providers, first separate the logic
> > performing the whitespace check into its own script. In subsequent
> > commits, this script is further generalized allowing its reuse.
> > 
> > Helped-by: Patrick Steinhardt <ps@pks.im>
> > Signed-off-by: Justin Tobler <jltobler@gmail.com>
> > ---
> [snip]
> > diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh
> > new file mode 100755
> > index 0000000000..f57d1ff5f0
> > --- /dev/null
> > +++ b/ci/check-whitespace.sh
> > @@ -0,0 +1,74 @@
> > +#!/bin/bash
> 
> This needs to be either "/bin/sh" or "/usr/bin/env bash".

> 
> > +baseCommit=$1
> > +outputFile=$2
> 
> I think the script would be more flexible if we just didn't have an
> output file in the first place and handle redirection of the output at
> the caller's side. That for example also allows you to easily append to
> a potential output file.
> 
> Edit: I see you change this in the next patch, so this is okay.
> 
> > +url=$3
> 
> We should check that we got 3 arguments in the first place.
> 
> Edit: I see that you add those checks in the next commit, but it does
> leave the reader wondering at this point. Maybe we should have a strict
> check here and then loosen it in the next commit where you make it
> optional.