Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support more options to specify who should review #23

Open
yuichielectric opened this issue May 14, 2021 · 4 comments
Open

Support more options to specify who should review #23

yuichielectric opened this issue May 14, 2021 · 4 comments
Assignees

Comments

@yuichielectric
Copy link
Contributor

Currently we only support required_reviewers, but Zappr has more options described here: https://zappr.readthedocs.io/en/latest/setup/

@dchomh dchomh self-assigned this May 17, 2021
@dchomh
Copy link
Contributor

dchomh commented May 17, 2021

I will look into this and try to organize the current feature set of Zappr so we can tackle the simplest ones first.

@dchomh
Copy link
Contributor

dchomh commented May 18, 2021

Feature set of Zappr:

  • Approval management
    • Minimum number of approvers
    • Approver ignore ability
    • Approve message pattern verification
    • Approver organization check
    • Approver username check
    • Approver group check/minimum number of approvers for group
  • Automatic branch creation per ticket
  • Commit message verification
  • Pull request format verification
  • Pull request label verification
  • Pull request task check

Current implementation of MVP:

  • Approval management
    • Minimum number of approvers
    • Approver ignore ability
    • Approve message pattern verification
    • Approver organization check
    • Approver username check
    • Approver group check/minimum number of approvers for group
  • Automatic branch creation per ticket
  • Commit message verification
  • Pull request format verification
  • Pull request label verification
  • Pull request task check

Out of the feature set given above, I believe we should focus on the features listed under approval management. I think the following order of implementation is a good start.

  1. Minimum number of approvers
  2. Approver organization check
  3. Approver ignore ability
  4. Approver group check/minimum approvers for group
  5. Approve message pattern verification (not sure if necessary)

Let me know if you have any thoughts on this.

@dchomh
Copy link
Contributor

dchomh commented May 20, 2021

Summary of discussion on 5/19:

Minimum number of approvers should be easy to handle, but can also be achieved via the existing protected branch mechanism.
Approver organization check should not be difficult to implement.
Main feature that would serve as a differentiator between the existing protected branch mechanism and this action would be the group check and minimum approvers for group.

Should try to see if developing this feature (number 4 in above list) is feasible before end of hackathon.

@yuichielectric
Copy link
Contributor Author

yuichielectric commented May 24, 2021

I have implemented the following options at #39 :

  • minimum at the top level
  • groups
  • groups > minimum
  • groups > from > users

Thanks to this PR, you can specify the following config, for example:

approvals:
  # check will succeed if there are 4 approvals from
  # backend or frontend people
  minimum: 2
  groups:
    # check will fail if there is not at least 1 approval
    # from backend persons
    backend:
      minimum: 1
      from:
        users:
          - yuichielectric
          - dchomh
    # check will fail if there is not at least 1 approval
    # from frontend persons
    frontend:
      minimum: 2
      from:
        users:
          - dchomh
          - rerwinx

This config requires

  • 2 approvals from someone at minimum, and
  • 1 approval from either yuichielectric or dchomh (or both), and
  • 2 approvals from both dchomh and rerwinx

#44 is a sample PR which checks the above config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants