On Fri, Feb 18, 2022 at 3:52 PM Petro Karashchenko < petro.karashche...@gmail.com> wrote:
> 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. > > All come from me, I think:(. All these patch meet the following criteria: 1. Approved by some people 2. Either the change is very minor 3. Or the PR was sent a few days ago. > 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 >
