Hi, I agree that auto-merge should not be used.
But I disagree that "as it is now since almost all patches follow the rule and seldom someone self-merges a patch". Here is a list of patches that were self merged last 12 days: https://github.com/apache/incubator-nuttx/pull/5474 https://github.com/apache/incubator-nuttx/pull/5445 https://github.com/apache/incubator-nuttx/pull/5444 https://github.com/apache/incubator-nuttx/pull/5428 https://github.com/apache/incubator-nuttx/pull/5425 https://github.com/apache/incubator-nuttx/pull/5508 All of the PRs have relatively low complexity and do not touch the core functionality so I'm ok with self-merging in such cases. Best regards, Petro пт, 18 лют. 2022 р. о 08:35 alin.jerpe...@sony.com <alin.jerpe...@sony.com> пише: > > Hi all > > In my opinion we should not use the auto merge functionality since most of > the time there is at least 1 of us active at any time and the amount of > patches is not comparable to EX: Google. > > I think that the merge policy is fine as it is now since almost all patches > follow the rule and seldom someone self-merges a patch. > > Also we should note that in case some patches land accidentally in the master > branch we can always revert them if it is necessary > > Best regards > Alin > > -----Original Message----- > From: David Sidrane <david.sidr...@nscdg.com> > Sent: den 17 februari 2022 22:31 > To: dev@nuttx.apache.org > Subject: RE: [DISCUSS]: Self merge and Single company/organization merge > gating > > On Self merge: > > As Nathan pointed out, it is more about time zones then merge velocity. > However, using a backport only methodology requires an upstream merge before > the work can be backported with least effort and adds a serial delay. It > would be ideal to reduces the CI quantum delay this as much as we can. > > GH has a setting to merge on successful CI after approval. It is lit by the > approver. This removes the polling for completion of CI. > If this can be configured it reduces the polling for both approver and > author. If it can not be configured in our repos, then self merge is the next > best thing. > > I am not trying to circumvent the review process at all - just remove the > idle time imposed by the process that is sampling related. > > > 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. > > I agree. > > David > > -----Original Message----- > From: Petro Karashchenko <petro.karashche...@gmail.com> > Sent: Thursday, February 17, 2022 1:01 PM > To: dev@nuttx.apache.org > Subject: Re: [DISCUSS]: Self merge and Single company/organization merge > gating > > 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
