Hi, On Fri, Mar 15, 2019 at 4:28 PM 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.
+1 to a label called NEED-MORE-REVIEW. Good idea! > > 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. +1 > > 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
