On 2020/4/2 9:02, Liyongle (Fred) via Tc wrote:
Hi all,
One colleague pinged me to discuss the process about review and approve. What he said was “PR的approve我们的流程是不是需要固定一下，好像感觉没有明确谁最终approve，大家都在看”. In English, it seems that many committers may have reviewed but no one approves usually, should we define who should approve?
For your information, currently, one PR will not be merged until both /lgtm and /approve are labeled. Only committers/maintainers can /lgtm and /approve.
In my opinion, it is up to each SIG to decide whether they need identify who should do /approve. In my team, we committers follow the non-written rule:
the PR should be reviewed, and all the comments should be handled and replied. If not, /lgtm will not be given.
if one committer gives the first /lgtm, the committer doesn’t give /approve usually, unless the PR is obviously an easy one. In other word, 2 committers to agree on the PR are a must.
What is your opinion or suggestion?
If there are multi maintainer for one SIG, I think,
- Should be a chief maintainer in the group, he needs to review/ack/nack/merge patches/pr when nobody else cared. - As you said, needs review/ack (/lgtm) from other maintainers before merged - But if nobody cared, the chief maintainer can decide to merge or not.
Thanks Hanjun