Support GH enterprise#12863
Conversation
Signed-off-by: jolheiser <john.olheiser@gmail.com>
GiteaBot
added
the
lgtm/need 1
6543
left a comment
6543
left a comment
There was a problem hiding this comment.
had tested this change in the past, it didn't worked on my test GHE
|
(cant test it now since I dont have this Test Instance anymore) |
|
|
||
| // NewGithubDownloaderV3 creates a github Downloader via github v3 API | ||
| func NewGithubDownloaderV3(ctx context.Context, userName, password, token, repoOwner, repoName string) *GithubDownloaderV3 { | ||
| func NewGithubDownloaderV3(ctx context.Context, baseURL, userName, password, token, repoOwner, repoName string) *GithubDownloaderV3 { |
There was a problem hiding this comment.
I think we could return with an error, so that we could know the error from NewEnterpriseClient
There was a problem hiding this comment.
The only error that NewEnterpriseClient throws is when parsing the given URLs, which have already been parsed prior to this func being called.
There was a problem hiding this comment.
So we assume NewEnterpriseClient will only return the parse error.
There was a problem hiding this comment.
Right, and it never should because we've already asserted the URL is parsable prior to calling this.
I can handle the error if it would make us more comfortable for future maintaining, I just left it out since we shouldn't run in to it.
Codecov Report
@@ Coverage Diff @@
## master #12863 +/- ##
==========================================
- Coverage 43.07% 43.07% -0.01%
==========================================
Files 658 658
Lines 72451 72454 +3
==========================================
+ Hits 31211 31212 +1
- Misses 36183 36185 +2
Partials 5057 5057
Continue to review full report at Codecov.
|
|
did someone test this patch sucessfully? |
|
I can test as soon as I can download an executable with the code somewhere. @zeripath told me #11815 (comment) that I can always get the lastest master to do that. And I did in #11815. But this code in question here is not yet merged to master. Is there some similar service building the PR branch executables, so that I can fetch an executable to test from there? |
|
@coldcoff if realy needed I can build the binary for you - witch OS & arch do you have? |
|
Played with it on the weekend (and yes: I was able to build this executable here for myself :-) )
Is the GitHub migration options expected to work on a mirror basis? |
No, mirrors don't pull over any items except for code. Would you mind opening an issue with your use-case? Currently all mirror migrations work that way. |
|
OK, without "this is a mirror" the migration from our GitHub Enterprise instance succeeded, see above! Will create a separate issue then for the other topic, thanks. |
GiteaBot
added
lgtm/done
lunny
added
type/feature
8 participants
I haven't had a chance to test with an instance of GH Enterprise, but I believe this should support it.
Unfortunately, the distinction made for the host is required because GH uses special URLs for their API
https://github.com/google/go-github/blob/dea2127aaa05fc2f42f9c137d87962f716e92414/github/github.go#L31-L32
However, from what I can tell, Enterprise should have sub-URLs 🤷♂️
https://github.com/google/go-github/blob/dea2127aaa05fc2f42f9c137d87962f716e92414/github/github.go#L301-L313
Giving it a sane base URL will cause the func to append the appropriate path
https://github.com/google/go-github/blob/dea2127aaa05fc2f42f9c137d87962f716e92414/github/github.go#L315-L339
Will fix #11815