Hello, Regarding PRs megre by the author: I think that if the changes are relatively simple (again that is very subjective, but I hope that people with merge rights have more or less the same common sense of it) and there is an approval from outside of the company/organization then the author can do the merge. For complex changes the person outside the organization should perform the merge even if there are more than 1 approval from inside the company/organization.
In this way reviewers can perform reviews with better quality and if someone "forget" to press the "rebase & merge" button because for example CI is still running and that is the end of working day, then the author can press that button and not do extra tagging in PRs. I vote to make that process usable for people and sacrifice bureaucracy in the places where it is possible. Best regards, Petro вт, 15 лют. 2022 р. о 18:26 Nathan Hartman <hartman.nat...@gmail.com> пише: > > On Mon, Feb 14, 2022 at 2:01 PM Brennan Ashton > <bash...@brennanashton.com> wrote: > > > Background: > > I am generally opposed to both of these. It is quite rare that we need a > > crazy fast merge turn around on a PR. And if something is approved and > > straight up broken in master that needs to get in then I think forgiveness > > can be used to self merge. > > > > > > I also generally do not have a big issue about people from the same company > > reviewing and merging. I could see the arguments for shared code but then I > > think we are nitpicking. I prefer the velocity with a few oops that can > > be reverted along the way if needed. There is also parts of the code base > > where the best people to review are on the same company. > > > > > > I think most of the concerns here are best addressed not by process but > > increasing the number of contributors who can participate. (more committers > > and PPMC) > > Feel free to correct me if I'm mistaken, but I think David is bringing > this up because of time zones. > > Indeed, most of the PR merging activity seems to occur during what I > would call nighttime or early morning, and I think that might be more > pronounced in David's time zone. > > Still, I think things have been working well, more or less, and I > don't think we need to make up any new rules right now. > > Instead, I would only urge committers to give complex PRs 12-24 hours > to percolate, even if there's an approving review, so other time zones > have a chance to look at them. > > Obviously that doesn't apply if it's urgent. For example, if the build > is broken and people can't get work done, or a serious error was > merged and needs to be reverted ASAP, don't wait, do it! > > Also, it's not necessary to delay for trivial PRs. > > What are the definitions of "complex," "trivial," "urgent," etc? I > say, committers should just use their best judgment and try to find a > good balance. Don't rush too much, don't delay too much. :-) > > David brings up a good point about time zones and we do have to > remember that NuttX is a global project, and I think that's the main > point to keep in mind. > > To Brennan's last point: as we grow the committer base, we are likely > to have more people in more time zones and more PR reviewers, so this > should become less of a concern. > > Nathan
