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

Create preview PRs using labels #1767

Merged
merged 1 commit into from
Apr 15, 2024
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Apr 11, 2024

Description of proposed changes

Benefits:

  1. Ease of use - straight from the PR page
  2. Workflow runs will show on the PR checks¹
  3. Downstream PRs will be automatically updated with new PR changes
  4. Simplified logic in reusable workflow

¹ In the case that a preview label already exists and another preview
label is added, this results in the workflow for the label that already
exists to appear as "skipped" because it has been skipped on the latest
event trigger to avoid an unnecessary update.

Checklist

  • Checks pass, though they don't test the changes here
  • If making user-facing changes, add a message in CHANGELOG.md summarizing the changes in this PR no functional changes
  • (to be done by a Nextstrain team member) Create preview PRs on downstream repositories.
  • PR previews are triggered on label
  • PR previews are auto-updated on new PR changes
  • Workflow runs show under PR checks
  • Dev docs updated

@victorlin victorlin self-assigned this Apr 11, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 22:49 Inactive
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from bad6acc to 249dfd8 Compare April 11, 2024 22:57
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 22:58 Inactive
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from 249dfd8 to 6669fcc Compare April 11, 2024 23:01
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 23:01 Inactive
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 23:03 Inactive
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from 860dede to 6669fcc Compare April 11, 2024 23:04
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 23:04 Inactive
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from 6669fcc to f55317a Compare April 11, 2024 23:12
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 23:12 Inactive
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from f55317a to a0d2ef3 Compare April 11, 2024 23:17
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 11, 2024 23:18 Inactive
@victorlin victorlin marked this pull request as ready for review April 11, 2024 23:25
@victorlin victorlin requested a review from a team April 11, 2024 23:25
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

I'm not the best person to give a thorough review of GitHub actions from a technical perspective, but the functionality looks great. I haven't seen this approach used before -- did you see it elsewhere or come up with it yourself?

@victorlin
Copy link
Member Author

I haven't seen this approach used before -- did you see it elsewhere or come up with it yourself?

The idea popped in my head while working on #1765. I searched online for "trigger on label" which led me to a StackOverflow example, and I took it from there.

Benefits:

1. Ease of use - straight from the PR page
2. Workflow runs will show on the PR checks¹
3. Downstream PRs will be automatically updated with new PR changes
4. Simplified logic in reusable workflow

¹ In the case that a preview label already exists and another preview
label is added, this results in the workflow for the label that already
exists to appear as "skipped" because it has been skipped on the latest
event trigger to avoid an unnecessary update.
@victorlin victorlin force-pushed the victorlin/preview-with-labels branch from a0d2ef3 to 90b7dee Compare April 15, 2024 18:15
@victorlin victorlin temporarily deployed to auspice-victorlin-previ-2nvl9v April 15, 2024 18:15 Inactive
@victorlin
Copy link
Member Author

I'll merge this now since this is a change on the dev side that we can iterate on.

@victorlin victorlin merged commit 0ca153a into master Apr 15, 2024
22 checks passed
@victorlin victorlin deleted the victorlin/preview-with-labels branch April 15, 2024 18:31
@tsibley
Copy link
Member

tsibley commented Apr 19, 2024

Did anyone ensure that labels can only be added by us and not random other GitHub users? Because I'm pretty sure anyone can add labels… and that's not what we want here.

@victorlin
Copy link
Member Author

That's a good concern, it hadn't crossed my mind. But I don't think anyone can add labels... I just tried on a random PR I had opened in bioconda-recipes and don't see a way to add a label other than tagging @BiocondaBot please add label which is something they had to strictly allow.

From GitHub docs:

Anyone with triage access to a repository can apply and dismiss labels.

@tsibley
Copy link
Member

tsibley commented Apr 19, 2024

Yay, that's great. I'm not sure why I recalled being able to add labels to issues/PRs I've created in third-party repos…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants