On 24/04/30 04:05PM, Patrick Steinhardt wrote: > On Tue, Apr 30, 2024 at 09:00:59AM -0500, Justin Tobler wrote: > > 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 > > Okay, fair enough. I'm still of the opinion that the infra here should > be shared. > > > > 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. > > Well, we could print the output to the console so that a user can see it > when they open the failed job. The nice formatting may be kind of moot, > but on the other hand it doesn't hurt, either. I guess most people are > used to reading plain markdown-style docs anyway. I'm thinking we can generalize the summary writing in some manner. When run as a GitHub action, the summary can be markdown formatted and written to `$GITHUB_STEP_SUMMARY` with no output to the console as done today. When run as GitLab CI, the summary can be written directly to console with links. All other runs just output normally to console. -Justin
