-
Notifications
You must be signed in to change notification settings - Fork 246
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
update readme for (gitlab) config #99
Conversation
@derimagia i didn't really expect "https://" to be in the config name, since that isn't part of the actual git repo URL. is it just a concession to keep the implementation simple? if possible seems like it wouldn't be part of the config name. wdyt? |
@paulirish I did that so we could take advantage of https://git-scm.com/docs/git-config#git-config---get-urlmatchnameURL (so I think you could even use it to do all subdomains as well?). I'm not too attached to it and am fine with changing it - it just looked like a good fit |
It looks like it also works with paths, so you can have it so that https://github.com/paulirish goes to one path but https://github.com and any other path goes to another one. I'd also mention in the readme that you can do --global. It looks like --local is the default for writing, so I think the way it was before is a little better ( |
Had the idea as well that we could take care of a lot of requests with this by also adding a setting for "open.branch" to override the branch like we do with protocol/server |
README.md
Outdated
git config [--global] gitopen.gitlab.ssh.port [value] | ||
``` | ||
<!-- | ||
git config --local --add open.[gitdomain].domain [value] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like --local is the default for writing, so I think the way it was before is a little better (git config [--global]
). Looks like --add isn't needed (I may have had it in the test, but looks like the default is "to replace at most one line").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya sgtm on dropping --local and --add. i also was headscratching on what value they added. :)
im ok with it, but it's just something we'll have to explain to the user, as its a bit counterintuitive. do you wanna take a swipe at updating these docs? I think you can speak to it a little better than me. :) |
4a337c9
to
e159276
Compare
Alright I made the basic change here of I left the https:// prefix the same. I think it probably need some additional explanation at some point, but I don't want to hold up the docs on it right now.
This is also something we could document if it comes up.
Seems interesting. If anyone is fond of the idea plz feel free to open an issue so we can explore it a bit more. |
this is docs for #92