-
-
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
Doc the fields of some libgit2 types #22543
Conversation
base/libgit2/types.jl
Outdated
* `version`: version of the struct in use, in case this changes later. For now, always `1`. | ||
* `abbreviated_size`: lower bound on the size of the abbreviated `GitHash` to use, defaulting to `7`. | ||
* `always_use_long_format`: set to `1` to use the long format for strings even if a short format can be used. | ||
* `dirty_suffix`: if set, if the [`workdir`](@ref) is dirty this will be appended to the description string. |
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.
reorder this? if set, this will be appended ... if the workdir is dirty
base/libgit2/types.jl
Outdated
be `GitHash(0)`. | ||
* `path`: a `NULL` terminated path to the item relative to the working directory of the repository. | ||
* `size`: the size of the item in bytes. | ||
* `flags`: a combination of the [`git_diff_flag_t`](https://libgit2.github.com/libgit2/#HEAD/type/git_diff_flag_t) flags. |
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.
combined how?
base/libgit2/types.jl
Outdated
* `size`: the size of the item in bytes. | ||
* `flags`: a combination of the [`git_diff_flag_t`](https://libgit2.github.com/libgit2/#HEAD/type/git_diff_flag_t) flags. | ||
* `mode`: the [`stat`](@ref) mode for the item. | ||
* `id_abbrev`: only present in versions newer than or equal to `0.25.0`. |
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.
libgit2 versions
Got everything? |
base/libgit2/types.jl
Outdated
|
||
The fields represent: | ||
* `version`: version of the struct in use, in case this changes later. For now, always `1`. | ||
* `proxytype`: the type of proxy to use. Default is to auto-detect it. |
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.
this could maybe use a xref to the enum that the values should come from
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.
there's also a Julia enum, right?
Again checked |
The fields represent: | ||
* `version`: version of the struct in use, in case this changes later. For now, always `1`. | ||
* `proxytype`: an `enum` for the type of proxy to use. | ||
Defined in [`git_proxy_t`](https://libgit2.github.com/libgit2/#HEAD/type/git_proxy_t). |
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.
does the Julia enum use the same spellings? worth spelling out the qualified Julia-side enum name those would be specified as
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.
Done - look ok?
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.
make check-whitespace
passed again locally
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.
looks great, though if it's not exported, maybe qualify it with the module name?
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.
If you're willing to wait, I hope to do another PR later tonight with examples that would show that it's not exported?
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.
ok
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.
Example added!
base/libgit2/types.jl
Outdated
flags. The `i`th bit of this integer sets the `i`th flag. | ||
* `mode`: the [`stat`](@ref) mode for the item. | ||
* `id_abbrev`: only present in LibGit2 versions newer than or equal to `0.25.0`. | ||
The length of the `id` field when converted using [`hex`](@ref). Usually equal to `GIT_OID_HEXSZ`. |
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.
is GIT_OID_HEXSZ a C name or the Julia enum name?
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.
For us it's OID_HEXSZ
. I'll update.
doc build is failing here, one of the ref's might not be getting resolved? |
Yeah, it was the link to |
@@ -351,6 +351,13 @@ These are used to select which global option to set or get and are used in `git_ | |||
SET_SSL_CERT_LOCATIONS = 12) | |||
|
|||
|
|||
""" | |||
Option flags for `GitProxy`. |
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.
hm this file is indented kind of strangely
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 sure is
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.
could fix it if you or someone else is feeling charitable, doesn't have to be in this pr tho
Anything else I should do? |
- `PROXY_SPECIFIED`: connect using the URL given in the `url` field of this struct. | ||
Default is to auto-detect the proxy type. | ||
* `url`: the URL of the proxy. | ||
* `credential_cb`: a pointer to a callback function which will be called if the remote |
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.
on the julia side this is just a function input, not a pointer right?
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, credential_cb::Ptr{Void}
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.
do you need to do something with cfunction
then to pass a julia function to it?
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.
Yeah I think you have to do something much like the stuff in base/libgit2/callbacks.jl
Anything else I should change? |
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.
superficially lgtm! :)
cf #18810