Skip to content

Comments

Enable the PR CI job to be run manually (#215).#216

Closed
ivanperez-keera wants to merge 1 commit intomainfrom
fix-pr-conditions
Closed

Enable the PR CI job to be run manually (#215).#216
ivanperez-keera wants to merge 1 commit intomainfrom
fix-pr-conditions

Conversation

@ivanperez-keera
Copy link
Contributor

The PR workflow CI job sometimes gets stuck, and there's no way to manually trigger it so that it completes. This prevents PRs from being merged.

This commit adjusts the conditions for the job to run so that it can be executed manually if needed.

Copy link
Contributor

@Bckempa Bckempa left a comment

Choose a reason for hiding this comment

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

I was kind of surprised to see the colons with no child attributes and since we don't have any child conditions of these triggers at the moment another valid syntax is on: [pull_request, workflow_dispatch], but if this works I'm happy with it.

How confident are we that a manual run will get picked up but a stuck PR?

Copy link
Member

@EzraBrooks EzraBrooks left a comment

Choose a reason for hiding this comment

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

This still puts the burden of the re-run onto the maintainers, since the average contributor can't hit this button. I would advise that we just use the typical strategy of putting up an empty commit temporarily to re-trigger the web hooks when we see GitHub lose an Action run.

@ivanperez-keera
Copy link
Contributor Author

This still puts the burden of the re-run onto the maintainers

I disagree. It doesn't put the burden on the maintainers, it gives them a possibility that otherwise they would not have.

If you add an empty commit, either you have to merge a PR with an empty commit, which is just noise, or when you remove it, the PR and the repo don't match (the PR doesn't really check a commit that was merged).

@EzraBrooks EzraBrooks dismissed their stale review April 8, 2025 19:31

I still disagree, but arguing would be a waste of time.

@EzraBrooks
Copy link
Member

You're still seeing runner brownouts because of #219, which I opened #220 to fix.

@ivanperez-keera ivanperez-keera force-pushed the fix-pr-conditions branch 2 times, most recently from a6e4de0 to 26f976c Compare April 15, 2025 18:09
@mkhansenbot
Copy link
Contributor

@ivanperez-keera - Can we merge this?

@ivanperez-keera
Copy link
Contributor Author

We've decided not to include this yet. We'll keep an eye to see if the problem really keeps happening and merge only in that case. Please do not scope this for any release yet.

The PR workflow CI job sometimes gets stuck, and there's no way to manually
trigger it so that it completes. This prevents PRs from being merged.

This commit adjusts the conditions for the job to run so that it can be
executed manually if needed.
@mkhansenbot
Copy link
Contributor

Can we close this? If we're not going to merge it, then I think we should.

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants