| Message ID | pull.776.git.git.1588432087854.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" | expand |
On Sat, May 02, 2020 at 03:08:07PM +0000, Johannes Schindelin via GitGitGadget wrote: > It was mentioned to me that it might not be totally helpful to run all > the builds whenever an in-progress branch is pushed to GitHub. For > example, if a contributor has dozens of topic branches in flight, they > might not want to have all of them built whenever a new end-of-day push > happens. As I was probably (one of) the mentioners here, thank you for looking into this. This definitely _helps_, and if you don't want to go further I could certainly make do with it[1]. But here are some thoughts on what a more ideal solution would look like (to me, anyway). What I'd _most_ like is a separately-maintained list of branches (or branch patterns) on which to run CI. I mostly care about just testing my personal equivalent of "next" (which is really "next" plus my stable topics), and I'd probably only list that branch. Plus potentially a special CI branch that I'd use when chasing down a failure that I can't reproduce locally. But I don't think there's any good way to implement that via GitHub Actions. I thought perhaps we could pull data from a different branch in the same repo, but referring to external "Actions" seems to require a full repo name (and obviously putting git/git in that name doesn't help anybody who's trying to override something in a fork). Another alternative: could we trigger CI based on branch-names? Locally, I call my unstable branches "jk/something-wip", and I'd like to skip all of the "-wip" branches. That's basically equivalent to "[skip ci]" in that I could just amend the tip commit of the -wip branches to say "skip ci". But what if we flipped the default to _not_ build? I.e.: - continue to build all pull requests; if you opened one, you're serious enough to have other people look and should get CI feedback - build branches with a few well-known names or patterns. Maybe names like "master"? Or maybe "refs/heads/build-ci/*"? Or maybe anything in "refs/heads/<initials>/*"? We'd have to decide on a convention as a community. - do not build anything else; we have no idea if somebody in a fork is just pushing up a work in progress or not, and they may be surprised to get a CI failure notification back. I'm not sure whether we want to be building all of the individual topics in gitster/git or not. In theory that provides more information, but I'm not sure if anybody is looking at them (and all of the notifications would go to Junio anyway). My ideas aren't really developed, but I guess what I'm wondering foremost is whether other people are thinking along the same lines. > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index fd4df939b50..0e4a280d309 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -7,6 +7,7 @@ env: > > jobs: > windows-build: > + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" It's unfortunate to have to repeat this in every job, as opposed to in the "on" event block, but I don't think that block is flexible enough to do what we want here. I don't know very much about Actions, but the code looks correct to me. -Peff [1] My current workaround is even more horrendous: I've turned off Actions entirely in peff/git, and then I separately push branches I want CI on into a separate repository. But that repo can't be a fork of git/git, because I already have one! So I now have an extra "git-ci" repo that isn't connected to anything. Yuck.
On 2020-05-03 05:36:46-0400, Jeff King <peff@peff.net> wrote: > On Sat, May 02, 2020 at 03:08:07PM +0000, Johannes Schindelin via GitGitGadget wrote: > > > It was mentioned to me that it might not be totally helpful to run all > > the builds whenever an in-progress branch is pushed to GitHub. For > > example, if a contributor has dozens of topic branches in flight, they > > might not want to have all of them built whenever a new end-of-day push > > happens. > > As I was probably (one of) the mentioners here, thank you for looking > into this. This definitely _helps_, and if you don't want to go further > I could certainly make do with it[1]. > > But here are some thoughts on what a more ideal solution would look > like (to me, anyway). > > What I'd _most_ like is a separately-maintained list of branches (or > branch patterns) on which to run CI. I mostly care about just testing > my personal equivalent of "next" (which is really "next" plus my stable > topics), and I'd probably only list that branch. Plus potentially a > special CI branch that I'd use when chasing down a failure that I can't > reproduce locally. > > But I don't think there's any good way to implement that via GitHub > Actions. I thought perhaps we could pull data from a different branch in > the same repo, but referring to external "Actions" seems to require a > full repo name (and obviously putting git/git in that name doesn't help > anybody who's trying to override something in a fork). > > Another alternative: could we trigger CI based on branch-names? Locally, > I call my unstable branches "jk/something-wip", and I'd like to skip all > of the "-wip" branches. That's basically equivalent to "[skip ci]" in > that I could just amend the tip commit of the -wip branches to say "skip > ci". But what if we flipped the default to _not_ build? I.e.: > > - continue to build all pull requests; if you opened one, you're > serious enough to have other people look and should get CI feedback > > - build branches with a few well-known names or patterns. Maybe > names like "master"? Or maybe "refs/heads/build-ci/*"? Or maybe > anything in "refs/heads/<initials>/*"? We'd have to decide on a > convention as a community. I think this is better approach for Junio, and we can help people opt in by asking them to push into "for-ci/<some-name>. This is my proposal for it: ------------------8<------------- From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= <congdanhqx@gmail.com> Date: Sun, 3 May 2020 18:56:50 +0700 Subject: [PATCH] CI: GitHub: limit GitHub Action to intergration branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b5..303393ba73 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,19 @@ name: CI/PR -on: [push, pull_request] +on: + pull_request: + branches: + - '*' + push: + branches: + - maint + - master + - next + - jch + - pu + - 'for-ci/**' + tags: + - '*' env: DEVELOPER: 1
Jeff King <peff@peff.net> writes: > I'm not sure whether we want to be building all of the individual topics > in gitster/git or not. In theory that provides more information, but I'm > not sure if anybody is looking at them (and all of the notifications > would go to Junio anyway). I do not think I am letting GitHub notifications for gitster/* come to my mailbox, so it is quite possible that many have piled up there without anybody (not even me) seeing them. I won't be locally able to notice breakages introduced by an individual topic to an environment I do not have access to, and building these topics with GitHub Actions may reveal which ones break, say, vsbuild, but I won't be debugging and fixing such issues myself anyway, so at least to me it is useless to run build there. I think I agree with your outline, i.e. build pull requests to help contributors, and build public integration branches to help the project as a whole. Thanks.
On Sun, May 03, 2020 at 07:05:46PM +0700, Danh Doan wrote: > This is my proposal for it: > [...] > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index fd4df939b5..303393ba73 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -1,6 +1,19 @@ > name: CI/PR > > -on: [push, pull_request] > +on: > + pull_request: > + branches: > + - '*' > + push: > + branches: > + - maint > + - master > + - next > + - jch > + - pu > + - 'for-ci/**' > + tags: > + - '*' Yeah, this seems quite reasonable to me. I'd just add a refspec to push a duplicate of branches I'm interested in running CI on to for-ci. -Peff
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b50..0e4a280d309 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,6 +7,7 @@ env: jobs: windows-build: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" runs-on: windows-latest steps: - uses: actions/checkout@v1 @@ -70,6 +71,7 @@ jobs: name: failed-tests-windows path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: MSYSTEM: MINGW64 NO_PERL: 1 @@ -154,6 +156,7 @@ jobs: ${{matrix.nr}} 10 t[0-9]*.sh) "@ regular: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" strategy: matrix: vector: @@ -189,6 +192,7 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} dockerized: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" strategy: matrix: vector: @@ -213,6 +217,7 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: jobname: StaticAnalysis runs-on: ubuntu-latest @@ -221,6 +226,7 @@ jobs: - run: ci/install-dependencies.sh - run: ci/run-static-analysis.sh documentation: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: jobname: Documentation runs-on: ubuntu-latest