Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: push trigger only on trunk branch and tags #101

Merged
merged 1 commit into from
Sep 27, 2023
Merged

ci: push trigger only on trunk branch and tags #101

merged 1 commit into from
Sep 27, 2023

Conversation

c-dilks
Copy link
Member

@c-dilks c-dilks commented Sep 16, 2023

If we are following trunk-based development with PRs, then only the trunk branch (development) should trigger on push.

Resolves #104

baltzell
baltzell previously approved these changes Sep 16, 2023
@baltzell baltzell dismissed their stale review September 16, 2023 23:09

Do we not want non-development branches running automatic tests?

@c-dilks c-dilks marked this pull request as draft September 18, 2023 15:44
@c-dilks
Copy link
Member Author

c-dilks commented Sep 18, 2023

I can't figure out the "best" way to do this such that everyone will be happy, but I would really like to stop the doubled workflow runs. After testing a bunch, I see 3 options for us to choose from:

1. Trigger on all PRs and all commits (what we currently do)

on:
  pull_request:
  push:
  • PRO: people working on "private" branches get CI triggers
  • CON: identical workflows run twice for every commit

2. Trigger on all commits always

on:
  push:
  • PRO: resolves doubled workflow issue, since no pull_request trigger
  • CON: workflow runs have no link back to the PR and instead just link to the commit; this is rather annoying, in my opinion

3. Trigger on PRs, and only on pushes to development

on:
  pull_request:
  push:
    branches: [ 'development' ]
  • PRO: resolves doubled workflow issue
  • CON: people working on "private" branches do not get workflow runs

After testing, I still prefer option 3. IMO collaborators should learn to always create PRs, whether they intend their changes to be merged or not (it's fine to create a test PR and close it). I would prefer draft PRs and frequent, smaller commits, rather than a sudden giant PR.

Changes should not be merged unless there is a PR, and tests will always run on a PR. If someone wants to wait until they are "done" to make a PR, and the tests suddenly fail, then that is their problem.

@c-dilks c-dilks changed the title ci: push trigger on trunk branch only ci: push trigger only on trunk branch and tags Sep 18, 2023
@c-dilks c-dilks marked this pull request as ready for review September 18, 2023 16:00
@c-dilks c-dilks marked this pull request as draft September 18, 2023 19:15
@c-dilks c-dilks marked this pull request as ready for review September 18, 2023 19:16
@baltzell
Copy link
Collaborator

Number 3 it is.

@c-dilks
Copy link
Member Author

c-dilks commented Sep 19, 2023

Number 3 is implemented, including tag triggers, so this is ready.

@baltzell baltzell merged commit e946395 into JeffersonLab:development Sep 27, 2023
7 checks passed
@c-dilks c-dilks deleted the ci-fix-trigger branch September 28, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow runs twice for pull requests
2 participants