If we cannot find a perfect way, then I think we should do it in the social way as Mark suggested.
Regards, -Ian. On Wed, Apr 3, 2019 at 1:09 PM Huxing Zhang <hux...@apache.org> wrote: > Hi All, > > On Wed, Apr 3, 2019 at 11:14 AM jun liu <ken.lj...@gmail.com> 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 <hux...@apache.org> wrote: > > > > > > Hi, > > > > > > On Wed, Mar 20, 2019 at 6:08 PM Mark Thomas <ma...@apache.org <mailto: > ma...@apache.org>> 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 <hyun...@apache.org> 于2019年3月20日周三 上午11:41写道： > > >>> > > >>>> Agree, we should have at least one approval before merging > > >>>> > > >>>> On 2019/03/20 03:00:51, Huxing Zhang <hux...@apache.org> wrote: > > >>>>> Hi, > > >>>>> > > >>>>> > > >>>>> On Wed, Mar 20, 2019 at 10:55 AM YunKun Huang <hyun...@apache.org> > > >>>> 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 <hux...@apache.org> 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 >
