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

Run dev workflow for external contributor PRs #11691

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Dec 12, 2023

Description

I noticed that #11678 and other external PRs do not have any GitHub Action workflows run. Nor do I see the approval workflow as described in Approving workflow runs from public forks.

While its not very clear from the GitHub actions documentation, it appears that the external contributor workflows are expected to be triggered from PRs rather than pushes.

This PR also cleans up some missed references when transitioning from Node 16 and 18 to Node 18 and 20 to reflect LTS status.

Issue number and link

N/A

Testing plan

This should:

  1. Trigger the dev workflow for this PR once opened
  2. Once merged into main, newly opened external PRs and their updates should trigger the dev workflow.

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change N/A
  • I have added or updated unit tests to ensure consistant code coverage N/A
  • I have update the inline documentation, and included code examples where relevant N/A
  • I have performed a self-review of my code
@ggetz
Copy link
Contributor Author

ggetz commented Dec 13, 2023

After speaking offline with @jjspace, we agreed on avoiding the double runs on PR updates where possible.

To avoid the double runs, we're going to run the deployment only for pushes (as these cannot be run by external contributors anyway), expect for those to the main or production branches. The the rest of the dev workflow will run on the pull_request trigger.

Ex.

on:
  push:
    branches:
      - main
      - production
      - "cesium.com"
  pull_request:
@ggetz
Copy link
Contributor Author

ggetz commented Dec 15, 2023

@jjspace Updated!

@ggetz
Copy link
Contributor Author

ggetz commented Jan 2, 2024

@jjspace Please review when you get the chance.

Copy link
Contributor

@jjspace jjspace left a comment

Choose a reason for hiding this comment

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

Just had a couple questions

.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/dev.yml Show resolved Hide resolved
.github/workflows/dev.yml Show resolved Hide resolved
@ggetz
Copy link
Contributor Author

ggetz commented Jan 8, 2024

@jjspace I answered your questions above. Is this OK to merge?

@jjspace
Copy link
Contributor

jjspace commented Jan 8, 2024

@ggetz Yes, I was looking in the wrong part of the docs. LGTM 🙏

@jjspace jjspace merged commit 6e8b5d8 into main Jan 8, 2024
9 checks passed
@jjspace jjspace deleted the actions-pull-request branch January 8, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants