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

On 24/05/03 08:56AM, Patrick Steinhardt wrote:

On Thu, May 02, 2024 at 02:38:38PM -0500, Justin Tobler wrote:

quoted

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 <redacted>
---
 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

quoted

+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.