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
Adds Gitlab provider #51
Conversation
a036a6d
to
2726861
Compare
2726861
to
a919cfd
Compare
This reverts commit 8dabacd.
Codecov Report
@@ Coverage Diff @@
## master #51 +/- ##
==========================================
+ Coverage 56.07% 58.85% +2.77%
==========================================
Files 24 38 +14
Lines 913 1694 +781
==========================================
+ Hits 512 997 +485
- Misses 361 571 +210
- Partials 40 126 +86
Continue to review full report at Codecov.
|
|
||
// Include scheme if custom, e.g.: | ||
// gitlabDomain = "https://gitlab.acme.org/" | ||
// gitlabDomain = "https://gitlab.dev.wkp.weave.works" |
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.
I might have added this comment here:
Include scheme if custom
But I don't think this should be the case. We shouldn't need to user to include the scheme when declaring custom domain vs. no-domain if using gitlab.com. wdyt?
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.
This was the original attempt, but it was added because of go-gitlab returning the error below when the custom domain was passed.
Unexpected error:
<*url.Error | 0xc0002aa4b0>: {
Op: "Get",
URL: "gitlab.dev.wkp.weave.works/api/v4/projects/dinos%2Ftestrepo",
Err: {
what: "unsupported protocol scheme",
str: "",
},
}
Get gitlab.dev.wkp.weave.works/api/v4/projects/dinos%2Ftestrepo: unsupported protocol scheme ""
occurred
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.
Good point. I guess it comes down to if we wanna support http
? Which as it hosted internally sometimes is "yes". But it would be nice to support scheme-less urls for "tidyness".
c, err = NewClient(gitlabToken, "", WithDomain("gitlab.works")) // "https://gitlab.works"
c, err = NewClient(gitlabToken, "", WithDomain("https://gitlab.works")) // "https://gitlab.works"
c, err = NewClient(gitlabToken, "", WithDomain("http://gitlab.works")) // "http://gitlab.works"
Could be a follow up issue.
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.
I agree, it could default to https
and mention it in the error message, only thinking if it's predictable. Will open an issue 👍
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.
💯 !
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.
Looks OK to me, but please note I am by no means an expert on this project.
Implements #8
This PR adds a Gitlab provider using the https://github.com/xanzy/go-gitlab client.
The mapping between gitprovider and Gitlab entities:
Specifics for this provider:
group/subgroup
.master
.