count github native approvals

[lotharschulz]

Zappr should count required github native reviews as well as it counts 👍s.

This way we can prepare zappr to incorporate github reviews in enforced rules.

[drummerwolli]

how is this different from #442 ?

[lotharschulz]

#442 is for sure related as well as rather old, I'd like to start from scratch.

[fokusferit]

This is basically my first task, I want to support the 'Approve' button from Github API as this will basically increase the whole experience with Zappr extremely :)

[fokusferit]

So I've started slowly on this topic and went through the code. Changing the approval workflow seems reasonable effort but I wouldn't do a breaking change (as stuff is running and people can't upgrade zappr 😄). So my thoughts are currently around how to support both cases and on the long-term deprecating "pattern" or allow both, as there might be teams who want both.
So this is my idea regarding adding a new flag to the zappr file:

diff --git a/config/config.yaml b/config/config.yaml
index 25bd8ca..ac21db2 100644
--- a/config/config.yaml
+++ b/config/config.yaml
@@ -77,6 +77,7 @@ ZAPPR_DEFAULT_CONFIG:
   approvals:
     minimum: 2
     ignore: none
+    default: false # to make it backwards compatible introducing default setting when true.
     pattern: "^(:\\+1:|<U+1F44D>)$"
     veto:
       # veto/blocking a PR = comment that matches this regex

I'm not completely happy with this but "default" (i hope) means here, teams want to use just the default github approval approach vs. own patterns. So you can't have both. This makes code changes easier. Maybe, developers don't want to pattern approach anyway and with this in the future we could reduce complexity.

Based on, I'm currently checking how code changes would look like and will provide a WIP PR soon. 🤞

And I went through the old issue and just checked again that for using this functionality repositories need to update their repo settings:

[lotharschulz]

Thanks for sharing your thoughts @fokusferit. I wonder if we find a better key label than default?
gh_native or github or gh might be alternative options.

---

I believe .zappr.yaml configuration like

....
  from:
    orgs:
      - zalando
....

must be respected when designing the "zappr required github native reviews feature" even with required approving reviews: 2 set. At least for Zalando it matters if approvers belong to Zalando organisations.

[fokusferit]

Definitely. I wouldn't change that. I would still keep this separate. The challenge will be more how to test it. I've already a PoC and would like to break and fix things (in tdd style).

[GeorgySerga]

Hey, seems like this issue is in stale mode for a long time, I'm going to attempt to fix it.

[GeorgySerga]

Hello @lotharschulz @drummerwolli @fokusferit , I forgot to leave a comment here after a PR above. It doesn't have tests (even fixes for broken tests), toggle or anything else except, what I think, a working logic to respect native approvals. I left some comments on the PR. Could you please check it, so I could change the approach or continue and finish it? Thanks!

[GeorgySerga]

Hello @rashamalek , seems like you might be one of the last active maintainers here, adding you to the thread to check it out. Thanks!

[rashamalek]

Thanks @GeorgySerga for contributing to Zalando/Zappr; @stoewer Could you please review the pr #586