DOC: Remove hint on PRs from origin/main #28638
Conversation
|
This didn't fail our pre-commit? |
|
you can either not installs it or explicitly skip it when committing (e.g. I'm currently getting spurious mypy failures via precommit locally (which I assume is mypy version issue due to virtualenv trying to be about 10% too clever and failing at its job) so I do every commit twice (once to let precommit fail and once skipping it to actually commit). |
It failed the local pre-commit. I had to deactivate it to be able to commit to main 😄. Putting it the other way round: Precommit makes the case of the hint even less likely. One more argument to remove it 😆 |
So I try and make sure anything we do as a pre-commit we somewhat document in our workflow, which may be how this came in in the first place 😅 But also I think we use hints way too liberally and Juanita's suggestion of moving this down to "open a pull request" as an optional type of thing makes sense & can be removed from here for now. Which the approval is self merge or move the sentence then self merge. 🤷♀️ |
|
The one way it can roll onto slightly more problematic for us as maintainers is that PRs from main do not allow us to push to a contributor's branch (because default branch protections are in place). In my experience this is true even if github says "Maintainers are allowed to edit this pull request" because that is a separate layer of protection. This isn't that much of a problem, but can mean a slight slow down for simple fixes we might otherwise just make and merge (e.g. typos etc). Just means we on occasion may have to wait for an extra round of the contributor responding, merge and open a followup, or open a PR from our own fork that branches from their main. It is relatively rare that we want to do all of that at once, but can happen. However, overall, I kind of doubt this particular note significantly decreases the prevalence of doing this. |
As poposed in #28575 (review), we don't need this. If developers have followed above instructions, they won't end up having changes on origin/main. But if they do, it's not really a problem on our side. We're still able to handle that. - As this PR shows, which I've intentionally made from my origin/main 😄 ping @story645
For an inexperienced user, it is simpler to continue with a PR from main rather than roll everything back and create a new PR.
The only problem exists on the user side in that they can't handle multiple concurrent changes when they only use main. But that's on them. And if they are inexperienced, they likely won't do that anyway.