| Message ID | 20240502193840.105355-5-jltobler@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add GitLab CI to check for whitespace errors | expand |
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. Patrick
On 24/05/03 08:56AM, Patrick Steinhardt wrote: > 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 Ya, good point. We should only accept 1 or 3 arguments as valid options. I'll update this. Thanks! -Justin > > +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.
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 +then + echo "USAGE: $0 <BASE_COMMIT> [<OUTPUT_FILE> <URL>]" + exit 1 +fi + problems=() commit= commitText= @@ -56,19 +67,29 @@ then goodParent=${baseCommit: 0:7} fi - echo "
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(-)