-
Notifications
You must be signed in to change notification settings - Fork 496
Allow for remote names other than origin
.
#2509
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
base: master
Are you sure you want to change the base?
Conversation
We introduce `getremotename` that tries to check the `remote.pushDefault` option to determine the remote name, and falls back to `origin` if it's not set.
Generally, I think trying to use
|
Thanks for the thoughts. I have changed the workflow to default to |
""" | ||
$(TYPEDSIGNATURES) | ||
|
||
Determines the GitHub remote of a directory, returninf 'origin' if that remote exists, |
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.
Determines the GitHub remote of a directory, returninf 'origin' if that remote exists, | |
Determines the GitHub remote of a directory, returning `origin` if that remote exists, |
@assert !isempty(pushdefault) | ||
@warn "Remote 'origin' not found. Using `git config --get remote.pushDefault` instead, which is set to '$(pushdefault)'." | ||
return pushdefault | ||
catch ; end |
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.
I suspect this PR needs to be reformatted with runic
try | ||
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir)) | ||
@assert !isempty(originurl) | ||
return "origin" | ||
catch; end |
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.
You seem to be using @assert
plus try/catch
for control flow. That seems like a bad idea for multiple reasons (e.g. future Julia version might allow disabling @assert
checks, which is, I think, mainly intended to enforce to check runtime invariants, which is not what this code does).
How about just e.g. this
try | |
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir)) | |
@assert !isempty(originurl) | |
return "origin" | |
catch; end | |
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir)) | |
if !isempty(originurl) | |
return "origin" | |
end |
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.
Same applies to the rest of the code, of course
@@ -616,7 +655,7 @@ it out automatically. | |||
`root` is the the directory where `git` gets run. `varname` is just informational and used | |||
to construct the warning messages. | |||
""" | |||
function git_remote_head_branch(varname, root; remotename = "origin", fallback = "master") | |||
function git_remote_head_branch(varname, root; fallback = "master") |
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.
The docstring still references remotename
. So either that needs to be adjusted. Alternatively, keep the kwarg here, but with a new default value nothing
; and then later only call getremotename
if remotename === nothing
?
We introduce
getremotename
that tries to check theremote.pushDefault
option to determine the remote name, and falls back toorigin
if it's not set.Fixes #2464 and perhaps #1916, as well as https://discourse.julialang.org/t/creating-documentation-when-remote-name-is-not-origin/110466.