Fix URL parsing not working when passing only a domain to GitlabHostname

[jocelynthode]

Fixes #377

I also added a unit test to check for this particular case.

[codecov]

Codecov Report

Merging #378 into master will decrease coverage by 0.04%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   70.36%   70.31%   -0.05%     
==========================================
  Files          62       62              
  Lines        3742     3743       +1     
==========================================
- Hits         2633     2632       -1     
- Misses        922      923       +1     
- Partials      187      188       +1

Impacted Files	Coverage Δ
server/events/vcs/gitlab_client.go	21.21% <80%> (+0.8%)	⬆️
server/events/runtime/plan_step_runner.go	91.54% <0%> (-2.82%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8c75f8...2adaf4b. Read the comment docs.

[jocelynthode]

@lkysow Sorry I seem to have been a little too eager in my PR. thanks for the pointer it is indeed way cleaner. I have amended my commit to take your changes into account.

While rereading this method however I feel like we should really in the future try to use a URL for gitlab (and maybe github as well) this would unify the behavior between Gitlab,Bitbucket and Github and I think would make it easier for the end user. Right now a user can either specify https://custom.domain or custom.domain, but if they want to use http then they are forced to specify the scheme while the variable is named "GitlabHostname" which implies we shouldn't use a scheme.

We could also simplify this part of the code by just always setting the GitLab API URL with the code instead of having an exception for "gitlab.com", since if the user puts "https://gitlab.com" it will run through the loop anyway.

What do you think ?

[lkysow]

• I agree that hostname is incorrect since a scheme and base-path can be set but I think it's okay to keep it for backwards compatibility sake and because it's not that big of a deal.
• I'd like to keep the if gitlab check because there's a bunch of stuff we only really want to do if it's a custom server. We don't need to test if gitlab.com is resolvable, nor do we need to custom configure the client so I'd rather not do all that stuff just for the sake of removing an if statement. When I read the code I'd wonder why we were doing all that stuff for gitlab.com

[jocelynthode]

@lkysow amended my commit with your requested changes.

• I understand that we want to keep it working this way for backward compatibility at the moment. However I think the behavior/naming should be changed in the future to avoid confusion for the user. For example, It could be made when having a major release that would include other breaking changes as well.
• Fair enough this was a little nitpick on my part.

In any case these the first point can be rediscussed in the future if more people also find the current behavior a bit confusing.

[lkysow]

Thanks!