Re: [PATCH 2/3] Makefile: use `find` to determine static header dependencies

[Jeff King <peff@peff.net>]

On Mon, Aug 25, 2014 at 04:00:42PM -0400, Jeff King wrote:

> On Mon, Aug 25, 2014 at 12:46:41PM -0700, Jonathan Nieder wrote:
> 
> > Jeff King wrote:
> > 
> > > -LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
> > > +LOCALIZED_C = $(C_OBJ:o=c) $(GENERATED_H)
> > 
> > Why is LIB_H dropped here?  This would mean that po/git.pot stops
> > including strings from macros and static inline functions in headers
> > (e.g., in parse-options.h).
> 
> Ick, this is an accidental leftover from the earlier iteration of the
> patch, which moved that part to a separate line (inside my gross
> GIT_REAL_POT conditional). The extra line went away, but I forgot to add
> $(LIB_H) back in here. Thanks for noticing.

As an aside, this makes an interesting case study for our git.git
workflow.

In some workflows, I would have made the original unacceptable patch,
you would have reviewed it, and then I would have made the followup
patch to adjust it, and you would have reviewed that. But it's quite
hard to see my mistake in just the followup patch; the fact that
$(LIB_H) was originally part of $(LOCALIZED_C) does not appear in that
hunk at all.

But in our workflow, we squash out the unacceptable diff, and you review
from scratch the movement from the original working state (assuming the
status quo was working :) ) to the newly proposed state. And there the
mistake is quite a bit more obvious.

Just an interesting observation.

-Peff