git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Chris. Webster via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Chris. Webster" <chris@webstech.net>
Subject: Re: [PATCH] ci: github action - add check for whitespace errors
Date: Tue, 22 Sep 2020 13:07:45 -0400	[thread overview]
Message-ID: <20200922170745.GA541915@coredump.intra.peff.net> (raw)
In-Reply-To: <pull.709.git.1600759684548.gitgitgadget@gmail.com>

On Tue, Sep 22, 2020 at 07:28:04AM +0000, Chris. Webster via GitGitGadget wrote:

> From: "Chris. Webster" <chris@webstech.net>
> 
> Not all developers are aware of `git diff --check` to warn
> about whitespace issues.  Running a check when a pull request is
> opened or updated can save time for reviewers and the submitter.

Sounds like a useful thing to have.

> A GitHub workflow will run when a pull request is created or the
> contents are updated to check the patch series.  A pull request
> provides the necessary information (number of commits) to only
> check the patch series.

I think this will work OK in practice, but a few thoughts:

 - for a linear branch on top of master, using the commit count will
   work reliably. But I suspect it would run into problems if there were
   ever a merge on a PR (e.g., back-merging from master), where we'd be
   subject to how `git log` linearizes the commits. That's not really a
   workflow I'd expect people to use with git.git, but it would probably
   be easy to make it more robust. Does the PR object provide the "base"
   oid, so we could do "git log $base..$head"?

 - this will run only on PRs. That's helpful for people using
   GitGitGadget, but it might also be useful for people just running the
   CI by pushing branches, or looking at CI builds of Junio's next or
   seen branches. Could we make it work there? Obviously we wouldn't be
   able to rely on having PR data, but I wonder if "git log
   HEAD..$branch" would be sufficient.

-Peff

  reply	other threads:[~2020-09-22 17:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-22  7:28 [PATCH] ci: github action - add check for whitespace errors Chris. Webster via GitGitGadget
2020-09-22 17:07 ` Jeff King [this message]
2020-09-22 17:55   ` Junio C Hamano
2020-09-22 22:41     ` Chris Webster
2020-10-09  5:00       ` Chris Webster
2020-10-09 13:20         ` Johannes Schindelin
2020-10-09 16:23           ` Junio C Hamano
2020-10-09 17:59             ` Jeff King
2020-10-09 18:13               ` Junio C Hamano
2020-10-09 18:18                 ` Jeff King
2020-10-09 18:56                   ` Junio C Hamano
2020-10-10  5:26                     ` Chris Webster
2020-10-10  6:29                       ` Junio C Hamano
2020-09-22 22:17   ` Chris Webster
2020-09-24  6:51     ` Jeff King
2020-09-25  5:10       ` Chris Webster
2020-09-25  6:44         ` Jeff King

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=20200922170745.GA541915@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=chris@webstech.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    /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 NNTP newsgroup(s).