BUPT / clubber.ml

A.M.C. - Artificial Intelligence & Machine Learning CLUB: Friends for Code, Paper, and Beers!🍻
https://ai-ml.club
Apache License 2.0
120 stars 37 forks source link

Discussion: merge right and responsibility #403

Open CyFeng16 opened 5 years ago

CyFeng16 commented 5 years ago

@hwfan What about we discussing the right and responsibility when dealing with merging PR? Then we can update the manuals and help improve the process.

hwfan commented 5 years ago

My idea is that the first reviewer comes is responsible for the whole process of merging. Other reviewers can comment as a supplement.(In order to avoid the mess of multi-processing)

huan commented 5 years ago

How about we change the rule to:

  1. Every PR needs two approvements before it is able to be merged.
  2. The second approver takes the responsibility to merge the PR.

Just my two cents, FYI.

CyFeng16 commented 5 years ago

Two-step reviewing sounds good to me.

CyFeng16 commented 5 years ago

@huan Could you please help modify the rules as you mentioned? Close this issue after you have modified the rules. Thanks a lot! :tada:

huan commented 5 years ago

@CyFeng16 It's just my suggestion.

I'd like to make it a discussing topic at one of our seminars, and write it in the meeting notes before we actually modify it.

Because I think it will be better than we modify it directly, how do you think?

CyFeng16 commented 5 years ago

@huan We are on the same page. I will submit a proposal at the next meeting.

CyFeng16 commented 5 years ago

BTW, voting for issues is really an essential feature.

CyFeng16 commented 5 years ago

@hwfan Have you got any conclusions on this issue at your last meeting(#406 )?