| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@ethomson : please take a look at this PR |
Sorry, something went wrong.
| * | ||
| * * opts(GIT_OPT_ENABLE_OFS_DELTA, int enabled) | ||
| * | ||
| * > Enable or disable ofs_delta capability. |
There was a problem hiding this comment.
I would add some more documentation here. Here's an example (though I'm not tied to this):
Enable or disable creating "offset deltas" when creating packfiles. Offset deltas store a delta base location as an offset into the packfile from the current location, which provides a shorter encoding and thus smaller resultant packfiles. Packfiles containing offset deltas can still be read. This defaults to enabled.
Sorry, something went wrong.
There was a problem hiding this comment.
👍 will update the comment
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, I now realize that I was rather misunderstanding the goal. It's not to create them, it's to request them to be created from the server (or not). In that case, given the broad name given to this option, I would say either:
This single option controls whether we create offset deltas and will override whether we request offset deltas. If we do this, I think that the documentation should read something like:
Enable or disable the use of "offset deltas" when creating packfiles, and the negotiation of them when talking to a remote server. Offset deltas store a delta base location as an offset into the packfile from the current location, which provides a shorter encoding and thus smaller resultant packfiles. Packfiles containing offset deltas can still be read. This defaults to enabled.
Separate options for each of these two scenarios. In that case, I would change the name of this option to something like GIT_OPT_ENABLE_REMOTE_OFS_DELTA, and document it something like:
Enable or disable requesting "offset deltas" when communicating with a remote. Offset deltas store a delta base location as an offset into the packfile from the current location, which provides a shorter encoding and thus smaller resultant packfiles. This defaults to enabled.
Note that if we go with (1), there's no more coding work to do... it appears that we simply don't support creating packfiles with ofs_deltas? (I didn't realize this until now, and somebody should sanity check me on it.)
Sorry, something went wrong.
There was a problem hiding this comment.
/cc @carlosmn who may have interesting opinions or insights here.
Sorry, something went wrong.
There was a problem hiding this comment.
our goal is actually (1) i.e. to simply not have any "ofs_delta".
does this name "GIT_OPT_ENABLE_OFS_DELTA" convey it properly ?
any better name suggestions?
Sorry, something went wrong.
There was a problem hiding this comment.
I think that this name is scoped nicely to (1), never create packfiles with offset deltas. I also think that this is the sensible scoping for this feature. As it happens, we will never write packfiles with offset deltas (this is still surprising to me!) so that's an easy win for that part of this support. 😀
Sorry, something went wrong.
| /* The minimal interval between progress updates (in seconds). */ | ||
| #define MIN_PROGRESS_UPDATE_INTERVAL 0.5 | ||
|
|
||
| bool git_ofs_delta__enabled = true; |
There was a problem hiding this comment.
This name doesn't quite make sense in our naming scheme -- we try to scope things to something sort of like an "object" or - when that stops making sense - the file level. (I concede that given the filename, smart_protocol.c, this has stopped making sense here, since this was logical outgrowth from smart.c which I guess just got too big). So git_smart is the object, or concept here, and it's the prefix. Anything that applies to that scope should be double underscored.
Something like: git_smart__ofs_delta_enabled.
(If you squint really hard it seems like we're trying to make C look object oriented, which is either terrible or merely odd depending on your mood.)
Sorry, something went wrong.
There was a problem hiding this comment.
ok, I was not aware of the naming scheme, will update it to git_smart__ofs_delta_enabled
Sorry, something went wrong.
This change provides an option in git_libgit2_opt_t which can be used in git_libgit2_opts to turn off/on ofs_delta capability in libGit2
|
closed and reopened to trigger a build because failure in appveyor build didn't looked to be related to my changes |
Sorry, something went wrong.
|
@ethomson : gentle ping on this, I have updated the comment and variable name. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This change provides an option in git_libgit2_opt_t which can be used in git_libgit2_opts to turn off/on ofs_delta capability in libGit2
This is related to Issue #1414 in libgit2Sharp repository
libgit2/libgit2sharp#1414