Trial run of adding newbase back #19970
Conversation
| Nullable{GitAnnotated}() | ||
| else | ||
| Nullable{GitAnnotated}(GitAnnotated(repo, newbase)) | ||
| end |
There was a problem hiding this comment.
This seems stylistically different than what's around elsewhere in Base (or maybe it's just that the else and end aren't in the same column that's bugging me). Perhaps something like
onto_ann = if isempty(newbase)
Nullable{GitAnnotated}()
else
Nullable{GitAnnotated}(GitAnnotated(repo, newbase))
endwould be acceptable? Not really a big deal.
There was a problem hiding this comment.
or
onto_ann = isempty(newbase) ? Nullable{GitAnnotated}() :
Nullable{GitAnnotated}(GitAnnotated(repo, newbase))
There was a problem hiding this comment.
Or even change the onto argument in GitRebase to accept a Union{Void, GitAnnotated}? I don't think the cost of dynamic dispatch is going to be to burdensome.
| @@ -457,7 +457,7 @@ function merge!(repo::GitRepo; | |||
| end | |||
|
|
|||
| """ | |||
| LibGit2.rebase!(repo::GitRepo[, upstream::AbstractString]) | |||
| LibGit2.rebase!(repo::GitRepo, upstream::AbstractString="", newbase::AbstractString="") | |||
|
|
|||
| Attempt an automatic merge rebase of the current branch, from `upstream` if provided, or | |||
| otherwise from the upstream tracking branch. | |||
There was a problem hiding this comment.
describe what newbase does here.
| Nullable{GitAnnotated}() | ||
| else | ||
| Nullable{GitAnnotated}(GitAnnotated(repo, newbase)) | ||
| end |
There was a problem hiding this comment.
Or even change the onto argument in GitRebase to accept a Union{Void, GitAnnotated}? I don't think the cost of dynamic dispatch is going to be to burdensome.
|
I'm a bit confused about how to test this, honestly. |
|
Ideally, I guess, we could compare the results to command line git. |
|
what does this correspond to in the libgit2 api? if they don't have it documented we can bug them about it |
|
Test added! |
|
@simonbyrne the git docs have a good explanation of what this does. I don't know if we want ASCII art in the docstring? |
Since these functions aren't exported from Base, I think they're more for people who know what they're doing. As such, I wouldn't think we need that level of detail in the docstrings. If anything, we could simply link to the relevant page in the Git documentation. |
|
|
||
| Attempt an automatic merge rebase of the current branch, from `upstream` if provided, or | ||
| otherwise from the upstream tracking branch. | ||
| `newbase` is the branch to rebase onto, or `NULL` to rebase onto the given `upstream`. |
There was a problem hiding this comment.
maybe:
`newbase` is the branch to rebase onto, by default this is `upstream`.
|
LGTM other than the doc wording. |
Addresses part of #19839
Addresses part of #19839