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

PR types to run actions #228

Merged
merged 7 commits into from
Jun 20, 2023
Merged

PR types to run actions #228

merged 7 commits into from
Jun 20, 2023

Conversation

halungge
Copy link
Contributor

@halungge halungge commented Jun 13, 2023

Github actions will not longer be triggered upon each sync of the PR, but only on these states:

  • opened
  • reopened
  • assigned
  • ready_for_review

My reasoning was: the author want so to have feedback when she opens the PR in the first place and when it is converted from draft -> full PR.

@halungge halungge marked this pull request as ready for review June 13, 2023 09:50
Copy link
Contributor

@jonasjucker jonasjucker left a comment

Choose a reason for hiding this comment

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

Thank you.
I will watch our billing-units in the next couple of days to see the impact.

@egparedes egparedes requested review from havogt and removed request for egparedes June 13, 2023 11:49
Copy link
Contributor

@havogt havogt left a comment

Choose a reason for hiding this comment

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

This will not run after pushes during review, i.e. no way to trigger before merge and therefore not possible to set-up branch protection rules (which we currently don't have). It seems with this setup, one might accidentally merge broken code.

Adding manual triggers + branch protection rules could solve that, but the manual trigger UI is not very handy (action -> select action -> run workflow -> select branch -> start).

@jonasjucker
Copy link
Contributor

@halungge
Copy link
Contributor Author

We could also add a pull_request_review trigger:

on:
  pull_request_review:
    types: [submitted, edited]

This would trigger a run by the action of the reviewer I guess not necessarily pushes to the repository.
or even on changes on individual PR comments with

on: 
  pull_request_review_comment:
    types: [created, edited]

@halungge
Copy link
Contributor Author

halungge commented Jun 14, 2023

And why not enable branch protection rules? Is there any reason we don't have it?
In icon-exclaim we use some kind of comment trigger with the launch jenkins comment, right? That is definitely nicer than triggering from the UI...

@jonasjucker
Copy link
Contributor

So any progress here?

If you don't find a solution I have to disable action for this repo to keep a minimum of resources for other repos.

@halungge halungge requested a review from jonasjucker June 20, 2023 12:00
Copy link
Contributor Author

@halungge halungge left a comment

Choose a reason for hiding this comment

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

check test fixes

@halungge halungge merged commit 92683bb into main Jun 20, 2023
@samkellerhals
Copy link
Contributor

This will not run after pushes during review, i.e. no way to trigger before merge and therefore not possible to set-up branch protection rules (which we currently don't have). It seems with this setup, one might accidentally merge broken code.

Adding manual triggers + branch protection rules could solve that, but the manual trigger UI is not very handy (action -> select action -> run workflow -> select branch -> start).

I agree here, currently in the situation where I would like QA and tests to run before merging...

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.

4 participants