From: Jonathan Tan <jonathantanmy@google.com>
To: matvore@comcast.net
Cc: jonathantanmy@google.com, matvore@google.com,
git@vger.kernel.org, sbeller@google.com, git@jeffhostetler.com,
jeffhost@microsoft.com, peff@peff.net, stefanbeller@gmail.com,
pclouds@gmail.com
Subject: Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
Date: Tue, 8 Jan 2019 15:19:45 -0800 [thread overview]
Message-ID: <20190108231945.36970-1-jonathantanmy@google.com> (raw)
In-Reply-To: <54fba0d3-4b8e-1faf-4b2d-e67c1f5fbf02@comcast.net>
> > Any reason for moving "case LOFS_BLOB" (and "case LOFS_BEGIN_TREE"
> > below) after LOFS_END_TREE?
>
> I put LOFS_BLOB and after LOFS_END_TREE since that is the order in all
> the other filter logic functions. I put LOFS_BEGIN_TREE at the end
> (which is different from the other filter logic functions) because it's
> usually better to put simpler things before longer or more complex
> things. LOFS_BEGIN_TREE is much more complex and if it were not the last
> switch section, it would tend to hide the sections that come after it.
>
> FWIW, I consider this the coding corollary of the end-weight problem in
> linguistics - see https://www.thoughtco.com/end-weight-grammar-1690594 -
> this is not my original idea, but something from the book Perl Best
> Practices, although that book only mentioned it in the context of
> ordering clauses in single statements rather than ordering entire blocks.
OK - my thinking was that we should minimize the diff, but this
reasoning makes sense to me.
> > Here, filter_trees_update_omits() is
> > only ever used to remove a blob from the omits set, since once this blob
> > is encountered with include_it == true, it is marked as LOFR_MARK_SEEN
> > and will not be traversed again.
> It is possible that include_it can be false and then in a later
> invocation it can be true. In that case, the blob will be added to the
> set and then removed from it.
Ah...yes, you're right.
> For your reference, here is an interdiff for this particular patch after
> applying your comments:
The interdiff looks good, thanks. All my issues are resolved.
next prev parent reply other threads:[~2019-01-08 23:19 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-10 23:40 [PATCH v2 0/2] support for filtering trees and blobs based on depth Matthew DeVore
2018-12-10 23:40 ` [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0 Matthew DeVore
2019-01-08 1:56 ` Jonathan Tan
2019-01-08 19:22 ` Matthew DeVore
2019-01-08 23:19 ` Jonathan Tan [this message]
2019-01-08 23:36 ` Junio C Hamano
2019-01-08 23:41 ` Jonathan Tan
2019-01-08 23:39 ` Junio C Hamano
2019-01-09 2:43 ` MATTHEW DEVORE
2018-12-10 23:40 ` [PATCH v2 2/2] tree:<depth>: skip some trees even when collecting omits Matthew DeVore
2019-01-08 2:00 ` Jonathan Tan
2019-01-08 23:22 ` Jonathan Tan
2019-01-09 2:47 ` MATTHEW DEVORE
2019-01-09 0:29 ` Matthew DeVore
2018-12-11 8:45 ` [PATCH v2 0/2] support for filtering trees and blobs based on depth Junio C Hamano
2019-01-08 0:56 ` Matthew DeVore
2019-01-09 2:59 ` [PATCH v3 " Matthew DeVore
2019-01-09 2:59 ` [PATCH v3 1/2] list-objects-filter: teach tree:# how to handle >0 Matthew DeVore
2019-01-09 2:59 ` [PATCH v3 2/2] tree:<depth>: skip some trees even when collecting omits Matthew DeVore
2019-01-09 18:06 ` [PATCH v3 0/2] support for filtering trees and blobs based on depth Jonathan Tan
2019-01-15 23:35 ` Junio C Hamano
2019-01-15 23:41 ` Junio C Hamano
2019-01-17 0:14 ` Matthew DeVore
2019-01-17 18:44 ` Junio C Hamano
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 \
--in-reply-to=20190108231945.36970-1-jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=jeffhost@microsoft.com \
--cc=matvore@comcast.net \
--cc=matvore@google.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sbeller@google.com \
--cc=stefanbeller@gmail.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.