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

Plugin modes for better MR handling. #252

Closed
wants to merge 3 commits into from

Conversation

pbendersky
Copy link

This PR enhances gitlab-plugin by adding two operation modes: Modern and Legacy.
Legacy is what we currently have, that works with older versions of GitLab.

Modern works with GitLab 8.1 or better, and provides an easier configuration of projects in Jenkins (a single Git repository is enough) and much better handling of Merge Requests (MR are fetched from origin and matched by SHA1 instead of comparing branch names).

This PR fixes the two issues I reported: #239 and #155.
Also, a quick review of the current issues indicates that it should fix (assuming they can use the Modern mode):

This has been implemented with a strategy pattern. The points where the strategy is used are ad-hoc based on where the Legacy and Modern strategies differ (a few but important points).

…acy mode, the plugin

behaves as usual, configured with multiple git repositores and a parametrized build.
In Modern mode, it fetches merge-requests from origin, so a simpler setup can be used (and indeed is recommended).
@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@cotman
Copy link

cotman commented Apr 1, 2016

This looks massively interesting to us, as we're impacted by one of the issues you refer to. A couple of questions, if that's OK (and this is the appropriate place for this):

  • Will this still play well with jobs that take a branch as a parameter now? As an example, we currently have single jobs that can be run manually by developers by launching the job in Jenkins and setting the gitlabSourceBranch parameter to the branch they want to test and that are also triggered via Gitlab webhooks automagically using the current Gitlab Plugin (which sets gitlabSourceBranch). If I'm reading this correctly then a default, empty, branch parameter (actual name of the parameter now wouldn't matter) would work for both cases using Modern mode i.e. developer populates the branch parameter for a manual build (unpopulated by default - the job is configured so that the Git plugin sets "Branches to build" based on it), webhook triggered builds have the branch deduced by the plugin, and work on the correct branch as a result?
  • Not totally related, but as the README.md has been updated - is it worth clarifying, a bit more, what "Use GitLab CI features" means with regard to how the Gitlab Plugin works? It might be just be me, but I've always been a little confused as to everything it offers (we always have it enabled). It appears under the Legacy mode with the updated README.md, so is no longer relevant for Modern mode?

Thanks.

@pbendersky
Copy link
Author

@cotman

  • That is correct. Setting a parameter with a default value of */master (Jenkins default) works as you expect: the web hook triggers builds normally for any branch with pushes / MR, and you can build manually and specify the branch to build (screenshots below).
  • The setting called "Use GitLab CI features" is probably a misnomer. It actually means "Use GitLab''s commit status API". When it's selected, the plugin will mark the builds in GitLab as passing or not. IMHO, having this granularity is useful only on Legacy mode, and on Modern mode it should be set by default. I left it as is since maybe there's some use case I'm missing where you want builds to trigger but the status not to be shown in GitLab.

screen shot 2016-04-01 at 9 59 49 am
screen shot 2016-04-01 at 9 59 58 am

@cotman
Copy link

cotman commented Apr 1, 2016

@pbendersky Excellent - thanks. Look forward to this making it towards the released plugin.

@pbendersky
Copy link
Author

@omehegan @coder-hugo not sure who are the maintainers of this project, but saw you have several commits. Is there anything I can do to help getting this merged?

@coder-hugo
Copy link
Contributor

@pbendersky I hadn't yet the time for a deeper look to your changes. But your branch is based on the current master which will be replaced in a few days by the release-1.2 brach which contains a lot of code cleanups (almost everything was touched during the cleanup). Therefore I'd suggest you to have a look to the release-1.2 branch and adapt your changes accordingly. This will definitively help getting it merged.

@pbendersky
Copy link
Author

@coder-hugo I'll look into release-1.2 and update this MR for it.

@pbendersky
Copy link
Author

@coder-hugo I see that you've released 1.2. I'll work into updating this MR for it.

@pbendersky
Copy link
Author

I just created #285 reimplementing this for 1.2 (BTW, the new architecture made it really easy to do, kudos). I'll close this PR, please keep the conversation in #285.

@pbendersky pbendersky closed this May 2, 2016
exceed-alae pushed a commit to exceed-alae/gitlab-plugin that referenced this pull request May 20, 2022
Update to latest from jenkinsci
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.

4 participants