>From Junio C Hamano, Mon 02 Mar 2020 at 10:57:16 (-0800) : > Missing full-stop (.) at the end. Oups. > > +--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. Yeah I was not too happy with my formulation either... > 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. Sorry. It looked to me like the documentation has an implicit wrap of around 78 characters, so I rewrapped the paragraphs accordingly. But you are right that since I was expecting comments on this patch anyway, I could have rewrapped only in future versions, to simplify the diff for this version. > 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 also a question of consistency. Some options are already documented as being passed to git-pack-objects. So when the user (ie me :)) sees an option in git-repack that also exists in git pack-objects, it is natural to assume it will be passed too, but since this is not explicitly stated for *this option*, whereas it is stated for *other options*, the user may wonder if this is truly the case. The user (still me :)) then looks at the source code to check, but would have appreciated if this was already in the documentation. > 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. And it also helps to illustrate how a 'porcelain' command like 'git repack' uses a plumbing command like 'git pack-objects'. > > `--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. So currently in the documentation of git-repack, for options passed to pack-objects, either 1) it is stated that this option is passed around and the option is not further documented (apart from refering to git-pack-objects(1)), eg -f, -F, -l 2) or the option is documented but git-pack-objects is not mentioned. So for options of type 2) I added a link to git-pack-objects. I agree with you that it would be good if options of type 1) were also documented in `git-repack`; it is annoying for an user to have to open another man page. I can do that in my next reroll. > > +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. The problem here is that `--keep-true-parents`, `--no-empty`, `--all`, `--reflog`, `--indexed-objects`, `--exclude-promisor-objects` are always passed and not driven by any options of `git repack`, so I did not know where else to put them. -- Damien Robert http://www.normalesup.org/~robert/pro
