CI: limit the on push triggers by XhmikosR · Pull Request #35574 · twbs/bootstrap

XhmikosR · 2021-12-20T12:36:22Z

Since now we have workflow_dispatch, we can trigger a workflow manually in case we don't have an open PR.

TODO:

Fix BrowserStack to run on PRs but only if we are on our repo and the actor has push rights

XhmikosR · 2021-12-21T07:13:04Z


 on:
  push:
+    branches:


We need to fix this to run on PRs if we are on our repo and the actor has push rights/is a member of the team, otherwise we should revert the BrowserStack change for now

If anyone has any ideas how to achieve this please let me know :)

Pinging @julien-deramond in case you have any suggestions about the above. Basically, because the secrets are not available on forks, we need to allow PR runs only for people with push rights. Or only for the main repositories branches.

I never managed to solve this so any help is greatly appreciated :)

you may want to put v4-dev , too

Pinging @julien-deramond in case you have any suggestions about the above.

Sent you a DM to have more details because I'm not sure to catch exactly the need.

But it seems that if: github.repository == 'twbs/bootstrap' already does the job.
When using a fork (people who doesn't have the push rights):

if the target is for example julien-deramond/bootstrap:main, Browserstack job is skipped in my forked Bootstrap repo

if the target is twbs/bootstrap:main, I don't see the Browserstack job run in my PRs on Bootstrap repo

Note: it would be possible to bypass that by modifying the workflow in a PR, but it is not common and still the secrets are not available.

Maybe something like that would fit the need:

on: push: branches: - main # runs only when code is pushed on the main branch - v4-dev # runs only when code is pushed on the v4-dev branch pull-request: branches: - main # run it when a PR has the main branch as a target (a PR with main-xmr-pa11y-ci as a target won't trigger it) - v4-dev # run it when a PR has the v4-dev branch as a direct target (a PR with main-xmr-pa11y-ci as a target won't trigger it) workflow_dispatch: # run it manually jobs: browserstack: if: github.repository == 'twbs/bootstrap' # the jobs are skipped when try to run it from a fork

If we want to be sure about all of that we could create a fake repo with some main and v4-dev branches and do some tests.

Thanks for the info! Indeed, I forgot I had the repo check.

So, I think the only thing left is to specify the pull_request branches, right?

Yes I think. And target v4-dev as well if you find it relevant.

I think v4-dev is enough if it's present in the v4-dev branch. We only need v4-dev for the CodeQL workflow's schedule event, I think (and it might be redundant there too).

Maybe https://github.com/actions-cool/check-user-permission could help on this now ?

mdo · 2022-02-25T17:03:10Z

Is this good to go @XhmikosR?

XhmikosR · 2022-03-01T15:15:27Z

I'm not sure I love this change hence why I'm hesitant. I removed this from 5.2.0 and we might find a better solution with using https://github.com/fkirc/skip-duplicate-actions

GeoSot · 2022-03-02T22:05:07Z

I'm not sure I love this change hence why I'm hesitant. I removed this from 5.2.0 and we might find a better solution with using https://github.com/fkirc/skip-duplicate-actions

I did a try on cspell, but I think we will agree, it is not the optimal result

Since now we have workflow_dispatch, we can trigger a workflow manually in case we don't have an open PR. Also, remove the custom `ci skip` code; it's supported natively for some time now: https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/

louismaximepiton

Hi, I don't know the actual priority of this PR, but here are my concerns + IMO, the merge of this PR would resolve the duped ci side effect when Bootstrap core team push on the repo (and might avoid the same issue on forked repo)

louismaximepiton · 2022-11-25T10:20:07Z


 on:
  push:
+    branches:


Maybe https://github.com/actions-cool/check-user-permission could help on this now ?

louismaximepiton · 2022-11-25T10:25:27Z

 on:
  push:
-    branches-ignore:
-      - "dependabot/**"


We tried something to remove dependabot from the CI (still actionnable by adding a tag on a dependabot branch). Added if: "!(github.actor == 'dependabot[bot]')" at each job level. Could be an alternative here if you don't want the action mentionned above. (https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions)

XhmikosR · 2022-12-16T07:14:56Z

Let's give this a go and see how everyone feels about it in practice.

My main concern is that I'd have to manually trigger actions for branches I don't have open PRs, but we'll see how it goes.

XhmikosR · 2022-12-16T07:32:13Z

OK, so this is causing BrowserStack to fire also on 3rd-party people PRs... I'm going to revert the whole PR for now, and we tweak it later.

This reverts commit f0ae5cc.

This reverts commit 71e2ace.

XhmikosR added v5 CI labels Dec 20, 2021

julien-deramond mentioned this pull request Dec 20, 2021

Enhance GitHub workflows (handle Dependabot, etc.) Orange-OpenSource/Orange-Boosted-Bootstrap#983

Merged

XhmikosR force-pushed the main-xmr-ci branch from 2d83fc4 to f70800a Compare December 21, 2021 07:08

XhmikosR commented Dec 21, 2021

View reviewed changes

XhmikosR force-pushed the main-xmr-ci branch from f70800a to 94fd48d Compare December 23, 2021 13:17

XhmikosR marked this pull request as ready for review January 29, 2022 14:20

mdo force-pushed the main-xmr-ci branch from 745c0e8 to 4f1b5bc Compare February 25, 2022 17:02

GeoSot force-pushed the main-xmr-ci branch 3 times, most recently from a948ee3 to 681aba3 Compare March 2, 2022 21:53

XhmikosR and others added 3 commits March 3, 2022 10:09

test skip-duplicated-actions

e341f10

Revert test skip-duplicated-actions

5e3c779

XhmikosR force-pushed the main-xmr-ci branch from 20f1a68 to 5e3c779 Compare March 3, 2022 08:10

Merge branch 'main' into main-xmr-ci

b2cfdf4

louismaximepiton reviewed Nov 25, 2022

View reviewed changes

Merge branch 'main' into main-xmr-ci

6046075

Merge branch 'main' into main-xmr-ci

941e4a3

XhmikosR merged commit f0ae5cc into main Dec 16, 2022

XhmikosR deleted the main-xmr-ci branch December 16, 2022 07:22

XhmikosR added a commit that referenced this pull request Dec 16, 2022

Revert "CI: limit the on push triggers (#35574)"

e9ae9d3

This reverts commit f0ae5cc.

XhmikosR mentioned this pull request Dec 16, 2022

Revert "CI: limit the on push triggers" #37657

Merged

XhmikosR added the skip-changelog So that the release drafter action doesn't include it label Dec 16, 2022

XhmikosR added a commit that referenced this pull request Dec 16, 2022

Revert "CI: limit the on push triggers (#35574)" (#37657)

71e2ace

This reverts commit f0ae5cc.

XhmikosR added a commit that referenced this pull request Mar 21, 2023

Revert "Revert "CI: limit the on push triggers (#35574)" (#37657)"

6b3374d

This reverts commit 71e2ace.

Uh oh!

Conversation

XhmikosR commented Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XhmikosR Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XhmikosR Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

XhmikosR Dec 23, 2021

Choose a reason for hiding this comment

Uh oh!

GeoSot Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

julien-deramond Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XhmikosR Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julien-deramond Jan 29, 2022

Choose a reason for hiding this comment

Uh oh!

XhmikosR Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louismaximepiton Nov 25, 2022

Choose a reason for hiding this comment

Uh oh!

mdo commented Feb 25, 2022

Uh oh!

XhmikosR commented Mar 1, 2022

Uh oh!

GeoSot commented Mar 2, 2022

Uh oh!

louismaximepiton left a comment

Choose a reason for hiding this comment

Uh oh!

louismaximepiton Nov 25, 2022

Choose a reason for hiding this comment

Uh oh!

louismaximepiton Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XhmikosR commented Dec 16, 2022

Uh oh!

XhmikosR commented Dec 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

XhmikosR commented Dec 20, 2021 •

edited

Loading

XhmikosR Dec 21, 2021 •

edited

Loading

julien-deramond Dec 24, 2021 •

edited

Loading

XhmikosR Jan 29, 2022 •

edited

Loading

XhmikosR Jan 29, 2022 •

edited

Loading

louismaximepiton Nov 25, 2022 •

edited

Loading