Hi All, On Wed, Apr 3, 2019 at 11:14 AM jun liu <[email protected]> wrote: > > > In some critical cases, one can approve the pull request by > > himself/herself with proper comments or commit directly. > > > I think the concern from Mark is quite reasonable, but this particular > approach Huxing suggests could solve it while at the same it gives the > community a technical rule to obey.
Actually I found this does not work as expected. I found that: - one cannot push to master branch directly - The one who raise the pull request can not merge the pull request by oneself. This will introduce the issue that Mark has pointed. If some critical cases, a second committer's approval is required. So do you think we should revert the changes? Or just keep it for a while? > > Jun > > > On Mar 20, 2019, at 10:45 PM, Huxing Zhang <[email protected]> wrote: > > > > Hi, > > > > On Wed, Mar 20, 2019 at 6:08 PM Mark Thomas <[email protected] > > <mailto:[email protected]>> wrote: > >> > >> I recommend that you enforce this socially rather than technically. > >> > >> There are times when a committer needs to get something committed (CI is > >> broken, security fix, correct an obvious bug that is blocking something > >> else, etc.) and having the flexibility to just commit it can be very > >> helpful. > > > > I think the discussion here does not prevent some one to commit > > directly. It is just about if a pull request is raised, it must be > > approved. > > In some critical cases, one can approve the pull request by > > himself/herself with proper comments or commit directly. > > > >> > >> Mark > >> > >> > >> On 20/03/2019 06:37, yuhang xiu wrote: > >>> agree. I think this is a very good way to help us standardize the pr > >>> merge. > >>> > >>> YunKun Huang <[email protected]> 于2019年3月20日周三 上午11:41写道： > >>> > >>>> Agree, we should have at least one approval before merging > >>>> > >>>> On 2019/03/20 03:00:51, Huxing Zhang <[email protected]> wrote: > >>>>> Hi, > >>>>> > >>>>> > >>>>> On Wed, Mar 20, 2019 at 10:55 AM YunKun Huang <[email protected]> > >>>> wrote: > >>>>>> > >>>>>> > >>>>>> I guess you are talking about "Require pull request reviews before > >>>> merging" feature [1] > >>>>> > >>>>> Cool! That is what I want. > >>>>> I am +1 to support enabling this and set the number of required approval > >>>> to 1. > >>>>> How do others think? > >>>>> > >>>>>> > >>>>>> [1] > >>>> https://help.github.com/en/articles/enabling-required-reviews-for-pull-requests > >>>>>> > >>>>>> On 2019/03/20 02:39:43, Huxing Zhang <[email protected]> wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> This pull request[1] is merged without any comments, or following any > >>>>>>> review process. > >>>>>>> Any ideas how to avoid this? > >>>>>>> Can Github support the feature that disable the merge button until a > >>>>>>> committer has approved the pull request? > >>>>>>> > >>>>>>> [1] https://github.com/apache/incubator-dubbo/pull/3693 > >>>>>>> -- > >>>>>>> Best Regards！ > >>>>>>> Huxing > >>>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Best Regards！ > >>>>> Huxing > >>>>> > >>>> > >>> > >> > > > > > > -- > > Best Regards！ > > Huxing > -- Best Regards！ Huxing
