Rxjs: Alternative code approval process

[kwonoj]

While current LGTM.co provides availability to protect codebase with strict approval system (that I'm in favor), it also exposes lacks of customization would better to have such as

• Different approval permissions per maintainers
  : Someone like full-committer can approve codes with single approval, while other maintainers requires N approval
• Allow / block self approval
• Other some good-to-have (do not allow to approve WIP PR, clear approval if PR's getting updated..)

Options can be considerd

1. Simply lower LGTM approval require to 1
2. Remove approval system
3. Moving LGTM to different approval checker (https://pullapprove.com/), support various customization mentioned in above

[kwonoj]

/cc @staltz @blesh @jayphelps .

I'd personally vote 3, which will mostly satisfies requirement as well as preserving explicit approval checker as well. (I believe 3 won't solve requiring rebase by protected branch though)

[jayphelps]

I vote for 3, if it truly does provide such ability. The rebase by protected branch shit is still a big issue in my eyes. I did some initial digging and couldn't find any solutions other than not using them.

[kwonoj]

For a reference, attaching docs http://docs.pullapprove.com/yaml/reviewers/ . It'll allows to setup reviewer group, each group can have different approval requirement.

[staltz]

I didn't know about pullapprove. Sounds ok to me.

[benlesh]

In the mean time I've already done this:

Simply lower LGTM approval require to 1

I think it'll be hard to get two of us to review _every_ PR.

[jayphelps]

pullapprove LGTM :shipit:

[kwonoj]

I'm verifying per-group permission to ensure to not cause daily flow churns - once verified will create PR shortly. Assigning myself for now.

[jayphelps]

I believe we actually CAN enable PR merges without requiring everyone to rebase even when there aren't conflicts! It's an option in Settings so an admin will have to change it (assuming everyone agrees)

Require branches to be up to date before merging

[kwonoj]

(assuming everyone agrees)

👍 ❤️

[kwonoj]

My current suggestion is https://github.com/zalando/zappr other than pullapprove, but main debate here will be

Do not use separate approval process (just use Github Review Approval) vs. use 3rd party , better than LGTM(which is gone now)

[jayphelps]

LGTM is now discontinued and no longer works, see: https://github.com/ReactiveX/rxjs/pull/2270

Now that GH supports native review/approvals, I think we should give that a spin before trying out any more third party solutions.

[kwonoj]

I'm closing this as inactivity. We can consider separate review process later.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.