-
Notifications
You must be signed in to change notification settings - Fork 619
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
Cleanup GitLabWebHook #207
Cleanup GitLabWebHook #207
Conversation
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
|
||
protected void buildOpenMergeRequests(final GitLabPushTrigger trigger, final Integer projectId, String projectRef) { | ||
try { | ||
GitLabConnectionProperty property = project.getProperty(GitLabConnectionProperty.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, you got it fixed here!
3303a7b
to
6b99f63
Compare
@slide, @markus-mnm: @omehegan told me that you are more experienced in Java/Jenkins plugin development than he. Could you please have a look to my changes before I merge them back? As you can see in the description of the PR this is based on another PR (which is also based on another PR 😉) So you can decide whether you want to review my whole changes or to review smaller changes by reviewing the PRs in that order: #192, #198, this one. Thanks, |
6b99f63
to
f2e0f27
Compare
…if addCiMessage was activated for the GitLabPushTrigger
…d for a given commit sha1 (fixes jenkinsci#159)
f2e0f27
to
3d533fd
Compare
Close this as there is no possibility to change the target branch. New PR #231 |
I'll have a look at this this weekend, but I might not provide as much value as one might hope. |
This PR cleans the code of the GitLabWebHook by splitting it into multiple classes.
The PR is based on #198.