What is our PR merge policy?

[egekorkan]

For node-wot, it seems that we had agreed on a policy that two technical leads (or committers?) of node-wot should approve a PR before merging. I think this can be a general Eclipse Thingweb policy but may need further thinking for cases like Playground where I am the only technical lead at the moment.

What are different thoughts?

P.S.: This can be an issue but I think it fits better for a discussion atm.

[relu91]

For node-wot the unspoken rule is more about two committers, but because the active committers group was small. I'm ok keeping this rule but, in my understanding, there is no such thing as a committer inside one single repository (per Eclipse rules). I could be very knowledgeable in node-wot and merge PRs in playground. Probably it would be better to define code owners or something similar 🤔 .

[danielpeintner]

The tricky part is that sometimes we have PRs that are very obvious and do not need further discussions. In such cases to wait for 2 committers to approve does not sound reasonable. Hence I think the code owners that are allowed to merge PRs have some freedom.
Having said that, in general it is a good rule to wait for 2 committers before merging a PR.

[JKRhb]

For node-wot the unspoken rule is more about two committers, but because the active committers group was small. I'm ok keeping this rule but, in my understanding, there is no such thing as a committer inside one single repository (per Eclipse rules). I could be very knowledgeable in node-wot and merge PRs in playground. Probably it would be better to define code owners or something similar 🤔 .

Maybe we could use the branch protection rule "Require review from Code Owners" (c.f. screenshot below) to enforce this? I guess in obvious cases, where changes are trivial and/or critical, we can also let committers bypass this rule, but as the number of projects (hopefully) continues to grow it might be good to at least have a confirmation step to prevent accidental merges, for example.