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
Feature/gitlab integration #11105
Feature/gitlab integration #11105
Conversation
26dcc7c
to
5690781
Compare
Can this be tried with public gitlab.com? |
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.
textboxes are wider than the the minimal width
very brief review but did not react to anything
Plugins/BuildServerIntegration/GitlabIntegration/ApiClient/GitlabApiClient.cs
Outdated
Show resolved
Hide resolved
Thank you for the contribution but I am really not excited about adding
another plugin in to the main repo.
My lack of excitement stems from several things:
1. Ownership - the Core team will essentially be responsible for
maintaining the plugin, triaging issues and fixing bugs after a short
while.
2. Release cadence - we're releasing the app few times a year, so any
issues with the plugin won't get addressed in timely manner. E.g., GitLab
changed its API, or bugs discovered in the plugin or few features
requested.
It'd be desirable if the plugin was released as a standalone plugin which
users can install via the Plugin Manager.
|
Thank you for feedback and review tips. As Gitlab mentioned in docs all significant API changes come major GitLab releases only. It should be very minor chance to receive unexpected failtures. The logic here is quite simple and similar to other integrations included in main repo. Can we discuss durability requirements which will make possible to merge it in product? |
The difference is if any maintainer use the feature or not. (I have no info if someone uses TeamCity or if the integration exists). It is quite frustrating to get issue reports for features you know nothing about and cannot test. Build server integration are quite special plugins, deeply integrated to GE so there are arguments to keep them in as well, then general cleanups are done as well. |
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.
{ | ||
public class GitlabPipeline | ||
{ | ||
public int id { get; set; } |
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.
@@ -0,0 +1,21 @@ | |||
using System; |
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.
Usings are not needed
Validates.NotNull(_apiClient); | ||
|
||
PagedResponse<GitlabPipeline> firstPage = await _apiClient.GetPipelinesAsync(sinceDate, running, 1); | ||
firstPage.Items.ForEach(item => ProcessLoadedBuild(item, observer)); |
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.
RussKie will ask this to changed to a normal loop
Sure, I tested on wireshark repo For settings use Instance URL https://gitlab.com/ For private projects token can be obtained according to this guide |
Some potential improvements:
So maybe it could be a good idea to set "https://gitlab.com" by default
If projectId is mandatory but not the token, for me it makes more sense to put the projectId field before the token one.
I would at least put a link to this documentation. But even better looking at the documentation, you can build a link open the url (using the server url) to let the user easily create a token. From the doc |
Philippe, thanks for your feedback! I'm going to fill URL and project ID right from remotes. |
Great! |
I'll try to lookup this way https://docs.gitlab.com/ee/api/projects.html#search-for-projects-by-name |
It's a little strange but with the request https://gitlab.com/api/v4/projects?search=wireshark&sort=asc I don't get the wireshark project in the list returned. We need a token to have the full list? Note: Maybe the field |
I'm away on holidays without an access to a computer (digital detox 😆). So
it may be some time before I can check it out.
|
Mine starting in 2 days 😆 |
You see paged response. The target one can be somewhere deep. |
Yes, so don't add too much complexity for something that is only made once and takes 2 minutes to do. |
The solutions was easier |
562e7b0
to
11a1caf
Compare
The ProjectId seems to be set for only one of the remotes. In my case I have "origin" with my changes and "upstream". The status is only shown for "origin". In addition, ProjectId is normally empty when opening the settings. I would like to say that the value is not read too, but it may be that I only got the build for origin but were looking for upstream builds. |
I've never worked in multiple remotes with configured CI/CD on different servers. Maybe I miss some important case, but anyway to support pipeline statuses from multiple origins we should allow to set several instance URLs. About ProjectId loading, it was bug, fixed :) |
Saving ProjectId seem to work for me now. My question was about multiple owners on the same server, origin and upstream. I actually use multiple servers too. Not just different URL but different implementations for different suppliers: Gitlab to BitBucket to GitHub and back again. @RussKie How to proceed here? Include this now and separate later? |
@e-stadnik Also; the build report tab page couldn't load the content since the inline browser is not supported by gitlab. Not a problem for me, the link to open it in an external browser works. |
@spdr870, Thanks a lot for you feedback! Can you see page numbers on pipelines web page for you project? If value for you project is below, plugin trying to request pages in parallel. In that case I'll try to change scheduler logic and it should helps with performance. As constant solution I think to cache loaded entities on disk because API will return updated entities. |
No pagenumbers, just Next. I manually altered the URL to find the number of pipelines, I could go to page 3342: |
Build status should be fetched and updated in the background (it is for at least Jenkins and AppVeyor).
The tab should be disabled by default for all build servers. |
…els/GitlabPipeline.cs Co-authored-by: Gerhard Olsson <6248932+gerhardol@users.noreply.github.com>
0f26a2e
to
571c8ad
Compare
I added BuildServer.Gitlab.PagesLimit settings but not sure about showing it in UI |
UI fields documentation added here: gitextensions/GitExtensionsDoc#148 @gerhardol sorry for force pushed commit, didn't expect that it'll break review comments. Hope everything will be fine for now. |
@RussKie I suggest that this plugin is included. Separating buildserver plugins may be done later. |
GetProjectIdLink.Location = new Point(82, 54); | ||
GetProjectIdLink.Name = "GetProjectIdLink"; | ||
GetProjectIdLink.Size = new Size(749, 15); | ||
GetProjectIdLink.TabIndex = 9; |
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.
Could you please confirm the tab order is logically sequenced?
Here's a test artifact: https://ci.appveyor.com/project/gitextensions/gitextensions/builds/48079386/artifacts |
I'd been testing build 4.2.0.17315 for a last 2 weeks on a private repo. On recent build checked tabIndex and pages loading limitation. Everything looks fine. |
I have been using this pr for some time too, but my interaction with gitlab is quite limited, so it does not push any limit. |
Thank you |
Fixes #11104
Proposed changes
Screenshots
Before
After
Test methodology
Test environment(s)
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.