Skip to content

Emoji approve #247

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

FlorianLudwig
Copy link
Contributor

First few bits needed for handling emoji to approve merge requests. Includes first steps for #146 and #145.

A few ideas on how to improve upon this:

1. approver eligability

I think it would be best to implement this on CODEOWNERS file in the repo. A quicker implementation might be to add this to the .marge-bot.yml:

approvers: # List of gitlab user names that are allowed to approve MR
 - joe_d
 - max_m

(which I did first but I prefer the CODEOWNERS solution so I did not publish it)

2. allow overwriting of all configs in .marge-bot.yml

As stated in #146 it would be great to overwrite the global config. But that would go in a separate merge request.

3. use marge-bot.yml from destination of merge request instead

Currently it uses the .marge-bot.yml config hardcoded from master, probably it would be better to use it from the merge request destination tree. Objections?

4. handle down votes

My first implementation did saw any 👎 as blocker not accepting a merge request as long as there is a single 👎. As that might not be sensible to do in every environment I decided to remove handling of 👎 at all for the merge request.

I think there might be three different ways to handle this:

# in any case only count thumbs of approvers that are configured to be allowed to do so.
approver_count_1 = thumbs_up - thumbs_down
approver_count_2 = thumbs_up if not thumbs_down else 0
approver_count_3 = thumbs_up # and ignore thumbs_down

It is a somewhat tricky topic so I guess it got to be configurable... :]

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

Successfully merging this pull request may close these issues.

1 participant