Skip to content

Conversation

@herbetom
Copy link
Contributor

@herbetom herbetom commented Jul 29, 2020

Since Gitlab 13.2 an approve and unapprove Action is available in Core: https://gitlab.com/help/user/project/merge_requests/merge_request_approvals#optional-approvals-core-only

The intent with this PR is to only use the Value form the HookAction if a value is found. Whether this is really what happens must be tested by someone. I have no environment in which I could easily test this.

@herbetom herbetom force-pushed the addApproveHookAction branch from ffbd80a to dcee48e Compare July 29, 2020 23:06
@herbetom herbetom changed the title Add approve and unapprove as possible actions gitlab: Add approve and unapprove as possible actions Jul 29, 2020
@mweinelt
Copy link
Contributor

mweinelt commented Jul 29, 2020

Does it make sense to fallback to the raw value and not map values where in == out? This has the added benefit to be more accepting of new states.

@herbetom
Copy link
Contributor Author

Yes, that makes perfect sense. It's just more "complicated" to implement.

@herbetom herbetom force-pushed the addApproveHookAction branch from dcee48e to dc59103 Compare August 5, 2020 18:28
@herbetom herbetom changed the title gitlab: Add approve and unapprove as possible actions gitlab: use map of HookActions only if value found Aug 5, 2020
@herbetom
Copy link
Contributor Author

herbetom commented Aug 5, 2020

@fleaz Could this work?

@herbetom herbetom changed the title gitlab: use map of HookActions only if value found gitlab: use HookActions map only if it is necessary Aug 5, 2020
@fleaz
Copy link
Owner

fleaz commented Aug 10, 2020

Hey @herbetom,
sorry for the delay.

This looks good to me, and also thanks to @mweinelt to the requested change.

Can you change your PR to target the development branch, than I will merge this 👍

@herbetom herbetom force-pushed the addApproveHookAction branch from dc59103 to dd0b3af Compare August 11, 2020 00:49
@herbetom herbetom changed the base branch from master to development August 11, 2020 00:49
@herbetom
Copy link
Contributor Author

I changed the target branch. We will see if it works 🤷‍♂️

@fleaz
Copy link
Owner

fleaz commented Aug 11, 2020

LGTM 👍

@fleaz fleaz merged commit 0cc6ecd into fleaz:development Aug 11, 2020
@herbetom herbetom deleted the addApproveHookAction branch October 24, 2020 14:18
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.

3 participants