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

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

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

quoted

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 [off-list ref]
Signed-off-by: Justin Tobler <redacted>
---

[snip]

quoted

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

Since the script is using some shell specific features, I'll update this
to "/usr/bin/env bash" in the next version.

quoted

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

quoted

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

For this patch specifically, I was trying to really only factor out the
whitespace check into its own script and keep changes outside of that to
a minimum. The next patch focuses on all the actual script changes and I
was hoping it might be easier to review that way. :)

-Justin