Re: [PATCH 2/2] gitlab-ci: add whitespace error check

> On 24/04/30 07:04AM, Patrick Steinhardt wrote:
> > On Mon, Apr 29, 2024 at 07:33:23PM -0500, Justin Tobler wrote:
> > > To check for whitespace errors introduced by a set of changes, there is
> > > the `.github/workflows/check-whitespace.yml` GitHub action. This script
> > > executes `git log --check` over a range containing the new commits and
> > > parses the output to generate a markdown formatted artifact that
> > > summarizes detected errors with GitHub links to the affected commits and
> > > blobs.
> > > 
> > > Since this script is rather specific to GitHub actions, a more general
> > > and simple `ci/check-whitespace.sh` is added instead that functions the
> > > same, but does not generate the markdown file for the action summary.
> > > From this, a new GitLab CI job is added to support the whitespace error
> > > check.
> > 
> > I still wonder whether we can unify these. Yes, the GitHub thing is
> > quite specific. But ultimately, what it does is to generate a proper
> > summary of where exactly the whitespaces issues are, which is something
> > that your version doesn't do. It's useful though for consumers of a
> > failed CI job to know exactly which commit has the issue.
> 
> Just to clarify, this new CI job still prints the output of 
> `git log --check` which details the exact commit and file with line
> number of the whitespace error. The difference is that it does not write
> an additional markdown file with links to the commit and blob.
> 
> Here is a failed execution of the GitLab whitespace check job:
> https://gitlab.com/gitlab-org/git/-/jobs/6749580210#L1289

> > So can't we pull out the logic into a script, refactor it such that it
> > knows to print both GitHub- and GitLab-style URLs, and then also print
> > the summary in GitLab CI?
> 
> We can do this, but for GitLab CI there probably isn't a point to
> generating a summary file since there is nothing that would pick it up
> and display it. Having links though directly in the job output would be
> nice. I'll give it another go.