godot-docs
godot-docs copied to clipboard
godotengine
Add PR review and merging guidelines for maintainers
This PR adds a set of guidelines and recommendation for reviewing and merging pull requests by maintainers and core contributors.
I'll keep fixes to separate commits so that the unresolved conversations are easier to track. I'll squash everything before the merge.
@hpnrep6 Thanks, though you probably should've waited until after we finalized the content and the wording. There are probably many changes incoming still.
Looks really good so far! I have two suggestions, both are procedural suggestions:
-
We should work on this in a shared doc with maintainers only as a first pass, that way we can quickly iterate on it.
-
We should wait until Akien is back before merging this. By merging this into the official docs we would be communicating that this is our preferred and accepted practice. However, as it stands right now, Akien has nearly all the responsibility for maintaining the engine and merging PRs, so this is a step away from the current process and represents a trial approach. IMO we shouldn't codify something in the docs until it because the accepted practice.
@clayjohn Yes, this of course needs Akien's approval. Though the merging part is mostly taken from his own ideas of what we should put on that checklist. But yeah, we wouldn't merge this doc without his input.
As for using something other than PR to iterate on it, this looks like a good idea, but I have nothing to suggest. Let's discuss it on RC.
Agree that this needs Akiens final approval. Iterating in a shared doc sounds like a good idea!
As mentioned in chat:
I believe we should add a policy to at least discourage merging your own PRs.
While it might be necessary from time to time, it is very easy to fall into bias.
If your PR has enough consensus and approval(s) it should be no problem to merging it.
I say "approval(s)" because 1 approval might be enough if it's from someone familiar with the code, but 2 might not be, if it's say from the "documentation" team and the "editor" team, but also touches things in "rendering" and there is no approval from that area.
In a few words, if you feel you have approval, and it should be merged, just ping the reviewers and ask them what's missing to merge your own beautiful PR.
