Re: [PATCH v2 4/5] ci: make the whitespace report optional

> On Thu, May 02, 2024 at 02:38:38PM -0500, Justin Tobler wrote:
> > The `check-whitespace` CI job generates a formatted output file
> > containing whitespace error information. As not all CI providers support
> > rendering a formatted summary, make its generation optional.
> > 
> > Signed-off-by: Justin Tobler <jltobler@gmail.com>
> > ---
> >  ci/check-whitespace.sh | 45 +++++++++++++++++++++++++++++++-----------
> >  1 file changed, 33 insertions(+), 12 deletions(-)
> > 
> > diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh
> > index f57d1ff5f0..fabd6ecde5 100755
> > --- a/ci/check-whitespace.sh
> > +++ b/ci/check-whitespace.sh
> > @@ -1,9 +1,20 @@
> >  #!/bin/bash
> > +#
> > +# Check that commits after a specified point do not contain new or modified
> > +# lines with whitespace errors. An optional formatted summary can be generated
> > +# by providing an output file path and url as additional arguments.
> > +#
> >  
> >  baseCommit=$1
> >  outputFile=$2
> >  url=$3
> >  
> > +if test "$#" -eq 0 || test "$#" -gt 3
> 
> That check is wrong, isn't it? Based on the usage below you either
> accept exactly 1 or exactly 3 arguments. But the condition here also
> accepts 2 arguments just fine. The following may be a bit easier to
> follow as it is more explicit:
> 
>     if test "$#" -ne 2 && test "$#" -ne 3
>     then
>         ...
>     fi

> > +then
> > +	echo "USAGE: $0 <BASE_COMMIT> [<OUTPUT_FILE> <URL>]"
> > +	exit 1
> > +fi
> 
> Ah, you make the output file optional here. Fair enough, then you can
> scratch that comment from my preceding mail as it did serve a purpose.