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

chore(ci): update CI to run on pull_request event #2943

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

rwaskiewicz
Copy link
Member

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe): Backport of fix(testing): puppeteer v10 support #2934

What is the current behavior?

Currently, GitHub Actions don't trigger for PRs created from forked repos, only when a push is made directly to the Stencil repo

What is the new behavior?

update CI to run on pull_request event to run on PRs from forked repos

Does this introduce a breaking change?

  • Yes
  • No

Other information

Runs your workflow when someone pushes to a repository branch, which triggers the push event.
(not particularly helpful on its own admittedly)

- update CI to run on pull_request event to run on PRs from forked repos
Comment on lines 3 to 9
on:
push:
branches:
- '**'
pull_request:
branches:
- '**'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying the branches is not necessary in this case:

Suggested change
on:
push:
branches:
- '**'
pull_request:
branches:
- '**'
on:
push:
pull_request:

Alternatively you can use the shorthand on: [push, pull_request].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with the shorthand, that 'felt more readable' to me

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm but it looks like as-is (I didn't think to check beforehand) these events aren't exclusive. IE right now, there's a pull_request and push set of checks running. Let me see what we can do about that

@rwaskiewicz rwaskiewicz force-pushed the rwaskiewicz/update-ci-branches branch from 28ed50c to d2a2d2e Compare June 28, 2021 13:22
@rwaskiewicz
Copy link
Member Author

@ltm I updated this to be explicit to run CI on our protected branches, and once on CI. I think that achieves what we effectively want - a clean CI run on the master (and v3.0.0-dev) branches every time we merge to them, but doesn't run both the ubuntu & windows builds 2x each on a PR. At this point, I think I prefer being "explicit" about branches in the pull_request event, even if it just is a glob. Let me know what you think.

@rwaskiewicz rwaskiewicz merged commit 5d78260 into master Jun 28, 2021
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/update-ci-branches branch June 28, 2021 13:57
rwaskiewicz added a commit that referenced this pull request Aug 17, 2021
- update CI to run on pull_request event to run on PRs from forked repos
- explicitly mark `push` events to prevent 2x runs when a PR is opened
   against a branch in the Stencil repo itself
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.

None yet

2 participants