Hi jun, Not a problem.
It’s just that we need to pay more attention in the future. :) kezhenxu94 <[email protected]> 于2019年3月15日周五 下午4:31写道： > 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. > > > Sorry again for making such mistake, I'll revert it if needed. > > > > > 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 > >> >
