Make remote URLs default to HTTPS #1690
Conversation
This makes Project.GitURL default to HTTPS instead of the plain git protocol. This function is also refactored to use early returns for readability's sake. I have changed all breaking tests to reflect the new behavior.
This makes `hub clone` use the HTTPS protocol by default instead of SSH. All breaking tests changed to reflect new behavior.
This makes `hub clone` clone over HTTPS for a private repository instead of SSH.
| if !isSSH && | ||
| args.Command != "submodule" && | ||
| !github.IsHttpsProtocol() { | ||
| isSSH = repo.Private || repo.Permissions.Push |
There was a problem hiding this comment.
Why did this logic go away? I imagine it should still be relevant if someone is explicitly configuring hub to use protocols other than https (if they want to keep old behavior)
There was a problem hiding this comment.
Hi @mislav, sorry for the delay. School's been busy!
This would be taken care of by Project.GitURL. If the hub.protocol is set to ssh or git, this would override the defaulting-to-https and use the user-defined protocol. Removing this block of logic just prevents the URLs from defaulting to ssh even if the repo is private or you have push permission. I felt that this would be in line with the spirit of #1596 for first-time users of hub to be able to use their GCM without any surprises. Should I put this logic back in?
| return fmt.Sprintf("git@%s:%s/%s.git", host, owner, name) | ||
| } | ||
|
|
||
| if preferredProtocol() == "git" { |
There was a problem hiding this comment.
If someone specifies git as their preferred protocol, does that still use ssh for "writeable" repos, e.g. cloning a repo to which you have writeable permissions? (This is due git protocol not being pushable.)
This would be a way people could opt into the old behavior before this change, but this behavior doesn't yet seem exercised in depth by updates to tests.
There was a problem hiding this comment.
Nope, if git is specified as the preferred protocol, all repos cloned will use that protocol regardless of their repo privileges/privacy. Wouldn't it be appropriate to respect that setting and clone using the plain git URL?
This would be a way people could opt into the old behavior before this change, but this behavior doesn't yet seem exercised in depth by updates to tests.
I'm not clear on what you mean. Are you referring to people who currently use hub, and currently set hub.protocol to git, and not surprising them with this change in behavior?
There was a problem hiding this comment.
Here's what I mean. We're about to make a change to one of the fundamental defaults of hub:
- Before:
gitprotocol is default, butsshis automatically used instead ofgitfor “pushable” repos, i.e. repos that you have write access to. - After:
httpsis used for everything.
If someone upgrades hub but wants to keep the old behavior, what configuration can they set to keep it?
My idea is that they would set git config --global hub.protocol git. Currently nobody uses hub.protocol set to git, because currently this option doesn't make sense since it's already the default.
There was a problem hiding this comment.
Hmm, okay, that sounds fair. This change should be made inside clone.go, right? Something like this to replace the block of logic I took out in clone.go?
if !isSSH &&
args.Command != "submodule" &&
github.IsGitProtocol() {
isSSH = repo.Private || repo.Permissions.Push
|
I know that sounds in a real world setting confusing to most computer geeks but actually to a accountant it makes great sense to me. It gives me the option to do so much more with my numbers at home because I only have one computer. Unfortunately, I am new to Github and all the computer protocols so it makes it difficult for me to branch things correctly in a systematic way. But give me a number and I will blow your mind. |
Fixes #1596 and closes #1318
This makes URLs default to using HTTPS instead of the plain git protocol (
git://), especially forhub clonewhich will not use SSH even for private repos/repos where you have push access to unless it is overridden by settinghub.protocoltossh.I think I got all the tests related to this change. Let me know if I missed out anything!