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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
> Signed-off-by: Justin Tobler <jltobler@xxxxxxxxx>
> ---
[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.

Patrick

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux