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

Editing a PR description shouldn't cause a CI run #836

Closed
sw opened this issue Apr 7, 2023 · 7 comments · Fixed by #1125
Closed

Editing a PR description shouldn't cause a CI run #836

sw opened this issue Apr 7, 2023 · 7 comments · Fixed by #1125
Labels
build Compilation issues

Comments

@sw
Copy link
Contributor

sw commented Apr 7, 2023

This seems excessive but I'm not sure how to turn that off (edited in build.yml?), without inadvertently also removing other triggers (such as a push to the branch the PR wants to merge).

Also, other actions such as requesting a review should not trigger a CI run IMO.

@sw sw added the build Compilation issues label Apr 7, 2023
@slaren
Copy link
Member

slaren commented Apr 7, 2023

I have noticed this too. Makes me feel a bit bad every time that I modify the description, but am not sure if this has any real cost for us anyway.

@anzz1
Copy link
Contributor

anzz1 commented Apr 8, 2023

build.yml

  pull_request:
-    types: [opened, synchronize, edited, reopened, review_requested, ready_for_review]
+    types: [opened, synchronize, reopened, review_requested, ready_for_review]

it was there so you can force a new CI run without pushing anything by editing the PR when it hangs due to github lagging but as you can also trigger that by other ways:

  • requesting a new review
  • mark as draft and back to ready for review
  • close and reopen

it's probably a good idea to remove the 'edited' especially since the CI run is rather big now

@sw
Copy link
Contributor Author

sw commented Apr 19, 2023

On the other hand, draft pull requests skip most of the CI checks, that's not so good.

@ggerganov
Copy link
Member

On the other hand, draft pull requests skip most of the CI checks, that's not so good.

I thought that usually drafts are incomplete and checks will likely fail, so I disabled them.
But don't have very strong feeling about this - we can reenable if people prefer it

@sw
Copy link
Contributor Author

sw commented Apr 21, 2023

I think you would fix the checks while in draft state, then set it to "ready for review" once they pass. But other people may have different habits.

I realize I can enable actions on my own branch, but ideally I would like to have them run only on manual trigger - that doesn't seem possible.

@sw
Copy link
Contributor Author

sw commented Apr 22, 2023

I would suggest the following:

  • disable the edited, review_requested, ready_for_review condition for pull requests
  • enable checks for drafts, so people get a chance for fixing failures before requesting reviews (yes, some checks may fail, but hopefully you're not hammering the git server with a thousand pushes, but test first on your machine)
  • prevent merge with failing checks

I believe at least the last one would have to be set by @ggerganov in the repository settings.

@ggerganov
Copy link
Member

Done

image

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

Successfully merging a pull request may close this issue.

4 participants