We'd better set up an official protocol for pull request that the whole community will honor and follow.
Thanks, -Ian. On Fri, Mar 15, 2019 at 5:18 PM Huxing Zhang <[email protected]> wrote: > On Fri, Mar 15, 2019 at 4:31 PM kezhenxu94 <[email protected]> wrote: > > > > Hi, > > > > > > > - The one who merged this pull request, just merge this pull request > > > without saying anything. I am not sure he mis-understand the > > > STATUS/READY-TO-MERGE label or not. But I think if read the context > > > through, one should be more cautious to merge that pull request. > > > > > > I'm so sorry it was me who merged that pull request. > > I had read through the context(actually I reviewed although I did not > comment) > > and indeed I've misunderstood the STATUS/READY-TO-MERGE label, > > and I suggest put more detail about the labels in the committer guide[1] > > to avoid later committers making the same mistakes. > > Big +1 to that. > > > > > > > Sorry again for making such mistake, I'll revert it if needed. > > I think there is no need to revert unless there is some serious issues > (not looking very deeply though), one can improve it by sending > another pull request if possible. > > > > > > > > > > > 1. > http://dubbo.apache.org/en-us/docs/developers/committer-guide/label-an-issue-guide_dev.html > > > > > > > > > > > > > > > > > > > > > > At 2019-03-15 16:28:16, "yuhang xiu" <[email protected]> wrote: > > >Hi, huxing. > > > > > >Great advice. In the previous review process, I also ignored some > issues. > > > > > >Maybe we need a new label such as: `need-more-review`. When we need more > > >reviews, we can mark this pr to remind other committers to review it. > > > > > >In addition, I think we need to clarify the meaning of > `READY-TO-MERGE`. I > > >think there is only one case where we will use this label, which is the > > >request that is approved, but this pr has not passed ci. > > > > > >In this pr merger, there are some problems, and we need to pay more > > >attention in the future. > > > > > >Huxing Zhang <[email protected]> 于2019年3月15日周五 下午4:03写道： > > > > > >> Hi, > > >> > > >> Looking at the reviewing process of the pull request, I can see some > > >> issues here: > > >> - No one has officially approved the pull request before it getting > > >> merged. By saying officially I mean this[2], which I've mentioned in > > >> another thread. > > >> - One of the reviewer has marked the milestone as 2.7.2, but applied > > >> it with a label called STATUS/READY-TO-MERGE. These two activities > > >> look contradictory but according to the context he's meaning I guess > > >> is to put some more time to it. > > >> - The one who merged this pull request, just merge this pull request > > >> without saying anything. I am not sure he mis-understand the > > >> STATUS/READY-TO-MERGE label or not. But I think if read the context > > >> through, one should be more cautious to merge that pull request. > > >> > > >> What I think can improve it: > > >> - Every pull request must be approved officially by at least one of > > >> the committer. (Comments such as LGTM does not mean official > > >> approval.) > > >> - If you feel uncertain about a pull request, please comment (see step > > >> 6 in [2]) that you need more review to come in to review or request > > >> review from others directly > > >> - If you want to merge the pull request, you must read through all the > > >> context. > > >> > > >> How do you think? > > >> > > >> [1] https://github.com/apache/incubator-dubbo/pull/3549 > > >> [2] > > >> > https://help.github.com/en/articles/approving-a-pull-request-with-required-reviews > > >> > > >> On Thu, Mar 14, 2019 at 10:03 PM Ian Luo <[email protected]> wrote: > > >> > > > >> > Folks, > > >> > > > >> > I think we need to revisit pull request 3549 in 2.7.2. In my > opinion, > > >> > fluent api need to be carefully resigned. > > >> > > > >> > What do you think? > > >> > > > >> > Thanks, > > >> > -Ian. > > >> > > >> > > >> > > >> -- > > >> Best Regards！ > > >> Huxing > > >> > > > > -- > Best Regards！ > Huxing >
