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

Normalize Gitlab urls #908

Merged
merged 6 commits into from
Jan 22, 2019
Merged

Normalize Gitlab urls #908

merged 6 commits into from
Jan 22, 2019

Conversation

morris25
Copy link
Contributor

Allows gitlab urls to be normalized by a separate protocol.

Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this—definitely good to have. I'm a bit bothered by how different the GitHub and GitLab logic appears to be at this point given that they seem like they should do very similar things. Is there any chance they can be made to mirror each other better or maybe even share logic?

src/GitTools.jl Outdated Show resolved Hide resolved
src/GitTools.jl Outdated Show resolved Hide resolved
src/GitTools.jl Outdated Show resolved Hide resolved
Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much cleaner! Thank you for the lovely refactor.

src/GitTools.jl Outdated Show resolved Hide resolved
src/GitTools.jl Outdated Show resolved Hide resolved
src/GitTools.jl Outdated Show resolved Hide resolved
@morris25 morris25 force-pushed the sm/normalize-gitlab branch from 71e65a6 to 3daa7b0 Compare November 19, 2018 15:58
src/Pkg.jl Outdated Show resolved Hide resolved
@omus
Copy link
Member

omus commented Nov 20, 2018

bors try

bors bot added a commit that referenced this pull request Nov 20, 2018
@bors
Copy link
Contributor

bors bot commented Nov 20, 2018

@fredrikekre
Copy link
Member

Would be great to get rid of the deprecations.

@StefanKarpinski
Copy link
Member

Ok, so I feel bad because I think I may have suggested the current API in this PR, but it seems like using keyword arguments might be better. In other words something like this signature:

setprotocol!(;
    protocol::Union{Nothing, AbstractString} = nothing,
    domain::Union{Nothing, AbstractString} = nothing,
    user::Union{Nothing, AbstractString} = (protocol == "ssh" ? "git" : nothing),
)

That doesn't quite feel orthogonal either though. Argh, this is a bit tricky...

Also, did we have setprotocol! in Julia 1.0?

@ararslan
Copy link
Member

Also, did we have setprotocol! in Julia 1.0?

Yes

@morris25 morris25 force-pushed the sm/normalize-gitlab branch from 8c9c706 to d3f03a7 Compare November 28, 2018 22:04
@morris25
Copy link
Contributor Author

@StefanKarpinski Is there anything I should do?

@StefanKarpinski
Copy link
Member

I think this looks good. Hopefully we'll use a centralized server with a consistent URL scheme instead in the future so that this becomes unnecessary. We'll discuss on the next Pkg triage call and make a decision on this.

@KristofferC
Copy link
Member

bors r+

@StefanKarpinski
Copy link
Member

Revisiting, this API seems ok with me.

bors bot added a commit that referenced this pull request Jan 8, 2019
908: Normalize Gitlab urls r=KristofferC a=morris25

Allows gitlab urls to be normalized by a separate protocol.

Co-authored-by: morris25 <sam.morrison@invenia.ca>
@bors
Copy link
Contributor

bors bot commented Jan 8, 2019

Build failed

@morris25 morris25 force-pushed the sm/normalize-gitlab branch from d3f03a7 to 8d1f449 Compare January 18, 2019 21:58
@KristofferC
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jan 22, 2019
908: Normalize Gitlab urls r=KristofferC a=morris25

Allows gitlab urls to be normalized by a separate protocol.

Co-authored-by: morris25 <sam.morrison@invenia.ca>
@bors
Copy link
Contributor

bors bot commented Jan 22, 2019

@bors bors bot merged commit 8d1f449 into JuliaLang:master Jan 22, 2019
@ararslan ararslan deleted the sm/normalize-gitlab branch January 22, 2019 20:35
@StefanKarpinski
Copy link
Member

Crap, this should have been squashed.

@KristofferC
Copy link
Member

:( the negative with Bors

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.

6 participants