-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add docs for GitRevWalker and push_head! #22654
Conversation
base/libgit2/walker.jl
Outdated
# Examples | ||
```julia | ||
oids = LibGit2.with(LibGit2.GitRevWalker(repo)) do walker | ||
LibGit2.map((oid,repo)->string(oid), walker, by = LibGit2.Consts.SORT_TIME) |
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 didn't use spaces around the =
in by
in the example below this one. It would be good to make them consistent, and in this case IMO it looks nicer without the spaces as it is in the second example.
base/libgit2/walker.jl
Outdated
|
||
A `GitRevWalker` *walks* through the *revisions* (i.e. commits) of | ||
a git repository `repo`. It is a collection of the commits | ||
in the repository, and supports iteration and calls to [`map`](@ref) |
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.
but LibGit.map
is distinct from Base.map
and has its own kwargs?
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 can add a docstring for LibGit2.map
, for some reason (???) I thought we already had one.
base/libgit2/walker.jl
Outdated
|
||
```julia | ||
cnt = LibGit2.with(LibGit2.GitRevWalker(repo)) do walker | ||
count((oid,repo)->(oid == commit_oid1), walker, oid=commit_oid1, by=LibGit2.Consts.SORT_TIME) |
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.
are oid
and by
keyword args specific to LibGit2.count
then or ... ?
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.
Guess this should get its own docstring too.
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.
and be module-qualified I guess if the example is intended to be runnable from outside the LibGit2 module (I hope LibGit2.count
isn't extending Base.count
with libgit2-specific kwargs?)
""" | ||
GitRevWalker(repo::GitRepo) | ||
|
||
A `GitRevWalker` *walks* through the *revisions* (i.e. commits) of |
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.
Maybe something like
A
GitRevWalker
is an iterable collection of the revisions (i.e. commits) of a git repositoryrepo
. Iteration over the collection walks the individual commits and returns(oid, repo)
tuples, whereoid
is a [GitHash
](@ ref) andrepo
is the given repository.Like many other such collections,
GitRevWalker
supports [map
](@ ref) and [count
](@ ref). The latter could, for example, be used to determine the number of commits in a repository made by a particular author.
Also TIL that LibGit2.map
is a thing. Do we have a docstring that points to that specifically?
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.
Nope! I can add one for this file
base/libgit2/walker.jl
Outdated
@@ -83,6 +82,27 @@ end | |||
|
|||
repository(w::GitRevWalker) = w.owner | |||
|
|||
""" | |||
LibGit2.map(f::Function, walker::GitRevWalker; oid::GitHash=GitHash(), by::Cint = Consts.SORT_NONE, rev::Bool=false) |
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.
Inconsistent spacing around =
. In this case I'd remove the spaces between Cint
and Consts
.
base/libgit2/walker.jl
Outdated
@@ -112,6 +132,30 @@ function Base.map(f::Function, walker::GitRevWalker; | |||
return res | |||
end | |||
|
|||
""" | |||
LibGit2.count(f::Function, walker::GitRevWalker; oid::GitHash=GitHash(), by::Cint = Consts.SORT_NONE, rev::Bool=false) |
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 comment regarding spacing
base/libgit2/walker.jl
Outdated
LibGit2.push!(w::GitRevWalker, cid::GitHash) | ||
|
||
Start the [`GitRevWalker`](@ref) `walker` at commit `cid`. This function can be used | ||
to apply a function to all commits since in a certain year, by passing the first commit |
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.
since or in? both seems redundant
Any ideas on how to fix the broken docs build? It's upset about the duplicate docstrings. |
add type constraints to some of them? |
I can try to fix this tonight but if someone else wants to try go ahead :) |
4710ad8
to
39bf802
Compare
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.
GitRevWalker Texas Ranger
Whoever merges this, please remember to squash. |
Reading the documentation is very confusing, because these functions don't extend the corresponding `Base` functions and they aren't exported. Ref: JuliaLang#22654 (comment).
Reading the documentation is very confusing, because these functions don't extend the corresponding `Base` functions and they aren't exported. Ref: #22654 (comment).
Reading the documentation is very confusing, because these functions don't extend the corresponding `Base` functions and they aren't exported. Ref: JuliaLang#22654 (comment).
Reading the documentation is very confusing, because these functions don't extend the corresponding `Base` functions and they aren't exported. Ref: JuliaLang#22654 (comment).
No description provided.