On Tue, Apr 30, 2024 at 07:04:46AM +0200, 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. > > 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? This was my feeling as well when I read the cover letter. I don't think we should either (a) have to remove functionality from the GitHub specific version of this job, or (b) have to maintain two different but highly similar implementations of the same job. It seems more reasonable that we'd do some third option, which is what Patrick suggests. Thanks, Taylor
