Re: [PATCH v4] documentation: add tutorial for revision walking

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 7, 2019, 19:19 UTC
Message-ID: <xmqqwofohjgv.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190806231952.39155-1-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 6 quoted lines

> Since v3, only a couple of minor changes from Jonathan Tan - thanks.
>
> I'm dropping the updates for the RFC set, since they're incremental from
> now. Next time you all see them they will be in a form which we would
> hope to maintain over a long period of time, checked into source -
> likely in the form of an mbox or dir of .patch file.

Sure.

> I think the tutorial itself is pretty much ready...

A few comments after skimming this round; none of them may be a show stopper, but others may have different opinions.

 - There is still a leftover "TODO: checking CLI options"; is that
   something we postpone teaching?

 - This is an offtopic tangent, but "my first contribution" being an
   addition of an entire command probably mistakenly raised the bar
   to contributors a bit too much.  A typical first contribution is
   a typofix, fix to a small (e.g. off-by-one) bug, etc.

 - For a revision walk tutorial, not seeing any mention of pathspec
   filtering and associated history simplification is somewhat
   unsatisfying.  On the other hand, I expect that enumeration of
   objects contained within commits is (hence various --filter
   options are) totally uninteresting for end users who run the
   command interactively and view the output of the command on
   screen.

 - Enumeration of objects is useful at least in three places in Git:
   (1) enumerate objects to be packed, with some filtering based on
   various criteria; (2) enumerate objects that are reachable from
   anchor points like refs, index, reflog, etc., to discover what
   are not reachable and can be discarded; (3) enumerate objects
   that still matter (i.e. the opposite of (2)) so that they can be
   fed to validation mechanisms (e.g. "cat-file --batch-check").  If
   this were titled "My first object enumeration", the reaction led
   to the latter half of the previous point may not have occurred
   (pathspec filtering would still be relevant, but not as
   much---for packing to create a narrow clone, you do not want to
   use pathspec with history simplification, but you would want to
   use something more like "root and intermediate trees that are
   necessary to cover these paths" filter in the
   list--objects-filter layer).

And from the point of view of the last item, I would think the new document is covering a need that is different from what we traditionally would call "revision walk", which is more about "git log", not the upstream of "git pack-objects", which this new document is more geared towards.

Unless "git walken" is an exercise of how to write code that does random thing, use of --grep filter however may be out of place, though. I do not offhand think of a use case where --grep would be useful in the revision walk/object enumeration that is placed upstream of "pack-objects".

Thanks.