From: Junio C Hamano <firstname.lastname@example.org> To: Damien Robert <email@example.com> Cc: firstname.lastname@example.org, Damien Robert <email@example.com> Subject: Re: [PATCH 1/2] doc: update the documentation of pack-objects and repack Date: Mon, 02 Mar 2020 10:57:16 -0800 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> (Damien Robert's message of "Fri, 28 Feb 2020 16:43:56 +0100") Damien Robert <firstname.lastname@example.org> writes: > diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt > index fecdf2600c..7f4923ddea 100644 > --- a/Documentation/git-pack-objects.txt > +++ b/Documentation/git-pack-objects.txt > @@ -80,6 +80,14 @@ base-name:: > as if all refs under `refs/` are specified to be > included. > > +--reflog:: > + This implies `--revs`. > + Include objects referred by reflog entries. > + > +--indexed-objects:: > + This implies `--revs`. > + Include objects referred to by the index > + Missing full-stop (.) at the end. > +--write-bitmap-index:: > + Write a reachability bitmap index as part of the pack. This > + only makes sense when used with `--all` and the pack is not > + outputted to stdout. Using "output" as a verb and conjugating it like this makes my head hurt. Let's instead borrow the phrase used in the description for the "--stdout" option, i.e. ... and the pack is not written to the standard output. > --keep-unreachable:: > - Objects unreachable from the refs in packs named with > - --unpacked= option are added to the resulting pack, in > - addition to the reachable objects that are not in packs marked > - with *.keep files. This implies `--revs`. > + Unreachable packed objects are added to the resulting pack. > + This implies `--revs`. Well spotted, and this update is very much appreciated. `--unpacked` does not take a name of a packfile at all, at least since 03a9683d ("Simplify is_kept_pack()", 2009-02-28). > @@ -88,13 +88,14 @@ to the new separate pack will be written. > > --window=<n>:: > --depth=<n>:: > - These two options affect how the objects contained in the pack are > - stored using delta compression. The objects are first internally > - sorted by type, size and optionally names and compared against the > - other objects within `--window` to see if using delta compression saves > - space. `--depth` limits the maximum delta depth; making it too deep > - affects the performance on the unpacker side, because delta data needs > - to be applied that many times to get to the necessary object. > + These two options are passed to `git pack-objects` and affect how > + the objects contained in the pack are stored using delta > + compression. The objects are first internally sorted by type, size > + and optionally names and compared against the other objects within > + `--window` to see if using delta compression saves space. `--depth` > + limits the maximum delta depth; making it too deep affects the > + performance on the unpacker side, because delta data needs to be > + applied that many times to get to the necessary object. It took me a while to realize that this only inserts "are passed to `git pack-objects` and" and does nothing else. It would have saved reviewers' time if the whole paragraph did not get rewrapped. I wonder if it helps the readers to tell the implementation detail (i.e. are passed to X) upfront like the updated text. It is true that it would help the interested readers who want to know _more_ to tell them that these corresponds to the options the underlying command has so they can go to the documentation of that other command and read more about them, though. > The default value for --window is 10 and --depth is 50. The maximum > depth is 4095. > @@ -103,13 +104,13 @@ depth is 4095. > This option is passed through to `git pack-objects`. > > --window-memory=<n>:: > - This option provides an additional limit on top of `--window`; > - the window size will dynamically scale down so as to not take > - up more than '<n>' bytes in memory. This is useful in > - repositories with a mix of large and small objects to not run > - out of memory with a large window, but still be able to take > - advantage of the large window for the smaller objects. The > - size can be suffixed with "k", "m", or "g". > + This option is passed to `git pack-objects` and provides an > + additional limit on top of `--window`; the window size will > + dynamically scale down so as to not take up more than '<n>' bytes > + in memory. This is useful in repositories with a mix of large and > + small objects to not run out of memory with a large window, but > + still be able to take advantage of the large window for the smaller > + objects. The size can be suffixed with "k", "m", or "g". Likewise. > `--window-memory=0` makes memory usage unlimited. The default > is taken from the `pack.windowMemory` configuration variable. > Note that the actual memory usage will be the limit multiplied > @@ -122,6 +123,7 @@ depth is 4095. > prevents the creation of a bitmap index. > The default is unlimited, unless the config variable > `pack.packSizeLimit` is set. > + This option is passed to `git pack-objects`. Here, you use a different way to add the information to help readers who would want to learn _more_. And I think this approach makes more sense than the previous two. All readers would appreciate if they can learn what they need to know to drive _this_ subcommand on the documentation page for _this_ subcommand without having to consulte another page, but those interested _can_ use reference like this. > @@ -129,7 +131,8 @@ depth is 4095. > only makes sense when used with `-a` or `-A`, as the bitmaps > must be able to refer to all reachable objects. This option > overrides the setting of `repack.writeBitmaps`. This option > - has no effect if multiple packfiles are created. > + has no effect if multiple packfiles are created, and is passed to > + `git pack-objects`. Ditto. > +Default options > +--------------- > + > +The command passes the following options to `git pack-objects`: > +`--keep-true-parents`, `--no-empty`, `--all`, `--reflog`, `--indexed-objects`. > +It also add `--exclude-promisor-objects` if there exists a promisor remote, > +and `--honor-pack-keep` except if `--pack-kept-objects` is passed. This is somewhat unconventional. I think we usually say, when describing each option --<option>, if it is enabled by default. I kind of like this sort of summary where options that are on by default can be seen in a single place, but (1) if we can reach a concensus that this is a good practice, we should do it in more places, and (2) if the sections for these individual options do not say that they are on by default, we should make them say so. Thanks.
next prev parent reply index Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-28 15:43 [PATCH 0/2] Documentation of pack " Damien Robert 2020-02-28 15:43 ` [PATCH 1/2] doc: update the documentation of pack-objects " Damien Robert 2020-03-02 18:57 ` Junio C Hamano [this message] 2020-03-03 17:41 ` Damien Robert 2020-03-03 18:49 ` Junio C Hamano 2020-03-03 21:23 ` Damien Robert 2020-03-03 22:29 ` Junio C Hamano 2020-03-12 17:09 ` [PATCH v2 0/3] Documentation of pack " Damien Robert 2020-03-12 17:09 ` [PATCH v2 1/3] pack-objects: change the name of add_objects_in_unpacked_packs Damien Robert 2020-03-12 17:09 ` [PATCH v2 2/3] doc: update the documentation of pack-objects and repack Damien Robert 2020-03-12 17:09 ` [PATCH v2 3/3] doc: add a short explanation for git-repack options Damien Robert 2020-03-25 22:15 ` [PATCH v2 0/3] Documentation of pack and repack Damien Robert 2020-03-27 22:21 ` Junio C Hamano 2020-02-28 15:43 ` [PATCH 2/2] pack-objects: change the name of add_objects_in_unpacked_packs Damien Robert 2020-02-28 16:01 ` Damien Robert
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Mailing List Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/git/0 git/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 git git/ https://lore.kernel.org/git \ firstname.lastname@example.org public-inbox-index git Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git