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

Supported RepositoryService.UpdateHook in GitLab driver. #73

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

Conversation

chhsia0
Copy link
Contributor

@chhsia0 chhsia0 commented Aug 21, 2020

Note that the returned hook events are changed to reflect the actual parameter names used to create or update the webhook. Specifically, the original convertEvents in the GitLab driver sets the value of Hook.Events with the
following mapping:

issues_events=true -> "issue"
tag_push_events=true -> "tag"
push_events=true -> "push"
note_events=true -> "comment"
merge_requests_events=true -> "merge"
other options -> none

It seems the event name strings are arbitrary and not consistent across different drivers, so the mapping itself doesn't provide much abstraction (if any). More importantly, for all other drivers, Hook.Events contains native events unrecognized by go-scm, but GitLab driver completely discards them. This is important to implement logics to "reconcile" webhooks (i.e., update only if configuration has changed) to sync the list of watched events. Therefore, this patch proposes Hook.Events returned by the hook-related functions in the GitLab driver to store the option names if their values are true, e.g., ["issues_events", "push_events", "confidential_issues_events"].

This is a breaking change.

Note that the returned hook events are changed to reflect the actual parameter
names used to create or update the webhook. Specifically, the original
`convertEvents` in the GitLab driver sets the value of `Hook.Events` with the
following mapping:
```
issues_events=true -> "issue"
tag_push_events=true -> "tag"
push_events=true -> "push"
note_events=true -> "comment"
merge_requests_events=true -> "merge"
other options -> none
```
It seems the event name strings are arbitrary and not consistent across
different drivers, so the mapping itself doesn't provide much abstraction (if
any). More importantly, for all other drivers, `Hook.Events` contains native
events unrecognized by go-scm, but GitLab driver completely discards them. This
is important to implement logics to "reconcile" webhooks (i.e., update only if
needed) to sync the list of watched events. Therefore, this patch proposes
`Hook.Events` returned by the hook-related functions in the GitLab driver to
store the option names if their values are true, e.g., `["issues_events",
"push_events", "confidential_issues_events"]`.

This is a breaking change.
@chhsia0 chhsia0 changed the title Supported RepositoryService.UpdatedHook in GitLab driver. Supported RepositoryService.UpdateHook in GitLab driver. Aug 22, 2020
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


chhsia0 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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