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

Proposal: remove branch-up-to-date requirement #3610

Closed
yurishkuro opened this issue Jul 18, 2023 · 8 comments
Closed

Proposal: remove branch-up-to-date requirement #3610

yurishkuro opened this issue Jul 18, 2023 · 8 comments
Labels
spec:miscellaneous For issues that don't match any other spec label triage:deciding:tc-inbox

Comments

@yurishkuro
Copy link
Member

What are you trying to achieve?

I often see a PR with required approvals ready to merge, but the merge button is disabled because the branch is not up to date. This leads to one of maintainers hitting Update Branch button and then forgetting about the issue until someone else comes across it and it's out of date with main again.

I propose to remove that up-to-date requirement. It makes much more sense in the code repositories, but for mostly text repos it does not provide a lot of benefit. GitHub won't allow to merge if there are conflicts, but aside from that merging slightly stale branch is usually not a problem.

@yurishkuro yurishkuro added the spec:miscellaneous For issues that don't match any other spec label label Jul 18, 2023
@Oberon00
Copy link
Member

The requirement was added to ensure that all linters/etc. currently on the main branch have run against the PR. #512 (comment)

@yurishkuro
Copy link
Member Author

@Oberon00 that sounds like it makes the process harder for everyone in perpetuity for the benefit of someone adding a new linter once a year. I see that pretty much everyone was agreeing with your original ticket to remove the restriction.

We can come up with another process for forcing all open PR to undergo a new linting rule, e.g. by closing/re-opening them once.

@yurishkuro
Copy link
Member Author

yurishkuro commented Jul 19, 2023

Another issue also in favor of non-blocking #936 where everyone agreed

@jmacd
Copy link
Contributor

jmacd commented Jul 19, 2023

I support lessening the impact of linters.

@yurishkuro
Copy link
Member Author

I see two issues with linters:

  1. a new linter is introduced, but the existing open PRs are not validated. I don't know exactly how GH behaves when a new required check is added - I think closing & reopening PR would make it subject to the new check, but it would begin failing consistently until the PR branch is rebased from main. GitHub does not provide this as a UI action when the protection in question is not available, which is quite annoying. A possible solution to this is having a bot in the repository that can react to a @rebase command, similar to how one can say @dependabot rebase.
  2. a linter logic is changed, e.g. a new rule is enabled. The old PRs would still pass the linting check because it will be old. One possible solution to this is changing the name of the check, e.g. to lint-something-v2, which would make the process follow as in bullet 1 above. Force-rebasing all open PRs (e.g. with a bot) would also solve it.

So I can see that there are indeed not straightforward issues, which if not address would create many other headaches if the uptodate requirement is disabled. It sounds like we need to solve them first.

Question: do we currently run any bots like that? I know some orgs like k8s do.

@austinlparker
Copy link
Member

@trask
Copy link
Member

trask commented May 7, 2024

(in particular)

Important: the only ones of these rules which may be changed are

...

  • Require branches to be up to date before merging
    • (this can also be unchecked)

@tigrannajaryan
Copy link
Member

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:miscellaneous For issues that don't match any other spec label triage:deciding:tc-inbox
Projects
None yet
Development

No branches or pull requests

7 participants