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

Gitlab integration should manage non default port #1099

Closed
jecisc opened this issue Dec 4, 2018 · 6 comments
Closed

Gitlab integration should manage non default port #1099

jecisc opened this issue Dec 4, 2018 · 6 comments

Comments

@jecisc
Copy link
Contributor

jecisc commented Dec 4, 2018

It would be great to be able to give a non default ssh port in a gitlab declaration.

I would like to be able to use a code like this:

Metacello new
	baseline: 'Bazard';
	repository: 'gitlab://gitlab.ferlicot.fr:3456:Projet/Bazard/src';
	load.

Or

Metacello new
	baseline: 'Bazard';
	repository: 'gitlab://gitlab.ferlicot.fr:3456/Projet/Bazard/src';
	load.

For than we need to update MCGitlabRepository class>>parseLocation:version: to extract the port then MCGitlabRepository>>scpUrl and MCGitlabRepository>>httpsUrl to use the port to get something like ssh://[email protected]:3456/Projet/Bazard.git instead of [email protected]/Projet/Bazard.git

@gcotelli
Copy link
Contributor

gcotelli commented Dec 4, 2018

@jecisc can you copy and paste here the SSH and HTTPS urls from the project page in Gitlab. I don't know it the port applies only to SSH url, or both.

@jecisc
Copy link
Contributor Author

jecisc commented Dec 4, 2018

@gcotelli I was tired and wrote HTTPS but the ssh port does not impact the HTTPS scheme I guess. I did a PR to Pharo and Iceberg to add the support already :)

pharo-project/pharo#2041
#1101

@gcotelli
Copy link
Contributor

gcotelli commented Dec 4, 2018

Nice, all the parsing in this hierarchy needs some love. I don't know if there's some case when the HTTPS url will include a port, in that case we will need some way to specify it and not confuse it with the SSH one. But for now the changes looks good to me.

@jecisc
Copy link
Contributor Author

jecisc commented Dec 4, 2018

I agree with you. But I'll let that to Cargo :P

@guillep
Copy link
Member

guillep commented Dec 18, 2018

Hi both :) What's the action to take here? I've relaunched the PR builds.
If that's ok, can I integrate this?

@gcotelli
Copy link
Contributor

I cannot test it but for me the changes are Ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants