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

Introduce a way to block PRs when the last daily test failed #1465

Open
madolson opened this issue Dec 20, 2024 · 9 comments
Open

Introduce a way to block PRs when the last daily test failed #1465

madolson opened this issue Dec 20, 2024 · 9 comments

Comments

@madolson
Copy link
Member

I think it would be useful to introduce some forcing function for us as a team to make sure the daily tests are staying green. My suggestion is we implement a check on the CI that looks into the last daily run and only allows code to get merged if it was green. @valkey-io/valkey-committers thoughts?

@ranshid
Copy link
Member

ranshid commented Dec 20, 2024

I think it is a good idea which will help motivate us to keep the daily green.
Why part of the ci though? maybe it is better to create another status check workflow which will use the github API to check the last daily status?

@hwware
Copy link
Member

hwware commented Dec 20, 2024

I agree with this, but I think we should tag the PR that we can merge but not merge due to CI issue. such as "To be merged" ?

@hpatro
Copy link
Collaborator

hpatro commented Dec 20, 2024

I guess we would need something like this https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue. Otherwise, it would be quite painful for the group to figure out when the daily tests are green/stable and figure out list of PR which can be merged.

@madolson
Copy link
Member Author

I'm not convinced a merge queue would solve this specific issue, but it's still something we've talked about adding quite often.

@hpatro
Copy link
Collaborator

hpatro commented Jan 2, 2025

I'm not convinced a merge queue would solve this specific issue, but it's still something we've talked about adding quite often.

I think it will act as a forcing function when most of the PR would start getting stuck in the queue.

@hpatro
Copy link
Collaborator

hpatro commented Jan 2, 2025

@madolson Do you have the permission to enable the merge queue functionality ?

@madolson
Copy link
Member Author

madolson commented Jan 4, 2025

Yes, I can give you permissions as well if you want to set it up.

@hpatro
Copy link
Collaborator

hpatro commented Jan 4, 2025

Yes, I can give you permissions as well if you want to set it up.

Sure, go ahead. I can try to set it up next week.

@madolson
Copy link
Member Author

madolson commented Jan 4, 2025

@hpatro You should have access to make changes now.

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

No branches or pull requests

4 participants