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

config: support insteadOf for remotes' URLs #79

Merged
merged 3 commits into from Dec 1, 2020

Conversation

@kostyay
Copy link
Contributor

@kostyay kostyay commented May 29, 2020

Added support for insteadOf configuration for remote's urls.
Can also be applied to submodules, but I didn't implement that.

This fixes: #76

@mcuadros
Copy link
Member

@mcuadros mcuadros commented May 29, 2020

Great! I have a big concern if you check how behaves git, with instead, it is straight forward. The insteadOf just happens in the clone operation, so once that the clone happens, the original URL is gone. So you don't need to keep track of it in the remote or something similar.

So I believe that we need to check the global/system configuration in the git.Clone and apply the URL, and the left the remote as it is right now.

Additionally, we should at an option in CloneOptions to allow enable or disable this behavior. (By default disable)

@kostyay
Copy link
Contributor Author

@kostyay kostyay commented May 29, 2020

ok, i will update the pr

@aca
Copy link

@aca aca commented Jun 30, 2020

@kostyay
This will be definitely useful. Any updates on this?

@kostyay
Copy link
Contributor Author

@kostyay kostyay commented Jun 30, 2020

I didn't have time to rewrite it yet.
I will see if I can get to it this weekend.

@kostyay kostyay force-pushed the kostyay:SupportUrlInsteadOf branch 2 times, most recently from 71d3f7f to 54b3d68 Jun 30, 2020
@kostyay
Copy link
Contributor Author

@kostyay kostyay commented Jun 30, 2020

@mcuadros I've updated the implementation following your comments.
Let me know what you think.

@kostyay kostyay force-pushed the kostyay:SupportUrlInsteadOf branch from 54b3d68 to 8193f4f Jun 30, 2020
@kostyay kostyay changed the title Support insteadOf for remotes' URLs [Updated] Support insteadOf for remotes' URLs Jul 7, 2020
@kamaln7
Copy link

@kamaln7 kamaln7 commented Dec 1, 2020

Hey, sorry that I'm being this person, but based on the discussion I feel like this slipped through the cracks. Any chance it could be reviewed again? Thank you!

@kostyay
Copy link
Contributor Author

@kostyay kostyay commented Dec 1, 2020

Hey, sorry that I'm being this person, but based on the discussion I feel like this slipped through the cracks. Any chance it could be reviewed again? Thank you!

The repo maintainer never bothered reviewing it

@mcuadros
Copy link
Member

@mcuadros mcuadros commented Dec 1, 2020

LGTM can you fix the conflicts @kostyay ?

@kostyay kostyay force-pushed the kostyay:SupportUrlInsteadOf branch from 8193f4f to 5db749a Dec 1, 2020
@kostyay
Copy link
Contributor Author

@kostyay kostyay commented Dec 1, 2020

Fixed

@kostyay
Copy link
Contributor Author

@kostyay kostyay commented Dec 1, 2020

Not sure why tests are failing.. flaky?

@mcuadros mcuadros changed the title [Updated] Support insteadOf for remotes' URLs config: support insteadOf for remotes' URLs Dec 1, 2020
@mcuadros mcuadros merged commit 51cbc24 into go-git:master Dec 1, 2020
6 of 8 checks passed
6 of 8 checks passed
@github-actions
version-matrix (1.13.x, ubuntu-latest) version-matrix (1.13.x, ubuntu-latest)
Details
@github-actions
test (master, ubuntu-latest)
Details
@github-actions
test (v2.11.0, ubuntu-latest) test (v2.11.0, ubuntu-latest)
Details
@github-actions
version-matrix (1.13.x, macos-latest)
Details
@github-actions
version-matrix (1.13.x, windows-latest)
Details
@github-actions
version-matrix (1.14.x, ubuntu-latest)
Details
@github-actions
version-matrix (1.14.x, macos-latest)
Details
@github-actions
version-matrix (1.14.x, windows-latest)
Details
@mcuadros
Copy link
Member

@mcuadros mcuadros commented Dec 1, 2020

Yup is a flaky test.

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

Successfully merging this pull request may close these issues.

4 participants