From: Justin Tobler <jltobler@gmail.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] gitlab-ci: add whitespace error check
Date: Tue, 30 Apr 2024 09:41:47 -0500 [thread overview]
Message-ID: <l55hxxuv5ohtno3gfjgq6mu7cqakfohon4vd7r535ztzadarin@iz4fbuwjeeug> (raw)
In-Reply-To: <ZjD6xNuncrq9fSXZ@tanuki>
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
next prev parent reply other threads:[~2024-04-30 14:43 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-30 0:33 [PATCH 0/2] Add GitLab CI to check for whitespace errors Justin Tobler
2024-04-30 0:33 ` [PATCH 1/2] ci: pre-collapse GitLab CI sections Justin Tobler
2024-04-30 0:33 ` [PATCH 2/2] gitlab-ci: add whitespace error check Justin Tobler
2024-04-30 1:59 ` James Liu
2024-04-30 3:01 ` Justin Tobler
2024-04-30 5:04 ` Patrick Steinhardt
2024-04-30 5:04 ` Patrick Steinhardt
2024-04-30 12:14 ` Taylor Blau
2024-04-30 14:00 ` Justin Tobler
2024-04-30 14:05 ` Patrick Steinhardt
2024-04-30 14:41 ` Justin Tobler [this message]
2024-04-30 14:45 ` Patrick Steinhardt
2024-04-30 14:58 ` Justin Tobler
2024-05-02 19:38 ` [PATCH v2 0/5] Add GitLab CI to check for whitespace errors Justin Tobler
2024-05-02 19:38 ` [PATCH v2 1/5] ci: pre-collapse GitLab CI sections Justin Tobler
2024-05-02 19:38 ` [PATCH v2 2/5] github-ci: fix link to whitespace error Justin Tobler
2024-05-02 19:38 ` [PATCH v2 3/5] ci: separate whitespace check script Justin Tobler
2024-05-03 6:56 ` Patrick Steinhardt
2024-05-03 15:27 ` Justin Tobler
2024-05-03 16:49 ` Junio C Hamano
2024-05-03 17:59 ` Patrick Steinhardt
2024-05-04 6:51 ` Chris Torek
2024-05-02 19:38 ` [PATCH v2 4/5] ci: make the whitespace report optional Justin Tobler
2024-05-03 6:56 ` Patrick Steinhardt
2024-05-03 15:35 ` Justin Tobler
2024-05-02 19:38 ` [PATCH v2 5/5] gitlab-ci: add whitespace error check Justin Tobler
2024-05-03 6:56 ` Patrick Steinhardt
2024-05-02 21:45 ` [PATCH v2 0/5] Add GitLab CI to check for whitespace errors Junio C Hamano
2024-05-03 15:39 ` Justin Tobler
2024-05-03 17:21 ` [PATCH v3 " Justin Tobler
2024-05-03 17:21 ` [PATCH v3 1/5] ci: pre-collapse GitLab CI sections Justin Tobler
2024-05-03 17:21 ` [PATCH v3 2/5] github-ci: fix link to whitespace error Justin Tobler
2024-05-03 17:21 ` [PATCH v3 3/5] ci: separate whitespace check script Justin Tobler
2024-05-03 17:21 ` [PATCH v3 4/5] ci: make the whitespace report optional Justin Tobler
2024-05-03 17:21 ` [PATCH v3 5/5] gitlab-ci: add whitespace error check Justin Tobler
2024-05-06 6:55 ` Patrick Steinhardt
2024-05-06 13:59 ` Justin Tobler
2024-05-06 17:17 ` Junio C Hamano
2024-05-06 19:21 ` Justin Tobler
2024-05-07 4:12 ` Patrick Steinhardt
2024-05-07 18:06 ` Justin Tobler
2024-05-07 18:12 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=l55hxxuv5ohtno3gfjgq6mu7cqakfohon4vd7r535ztzadarin@iz4fbuwjeeug \
--to=jltobler@gmail.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).