On 2013-02-03 21:42, Stefano Lattarini wrote:
> I've pushed the promised patches to the rewindable branch
> 'experimental/preproc' (based off of maint).  I'll also soon
> send them to the list to simplify review (I will drop the
> bug tracker from CC:, to avoid cluttering up the report).
> 
> As usual, reviews are welcome.

I like the end result of this series, I especially like that I don't have
to type &{this}& anymore. But I have some doubts whether the longish
way to get there is really all that interesting? The argument that you
should keep the history to show how you get from A to B is just not worth
it when it's just about bike-shedding the naming... Why not merge 1 with
the nobrainer renames of 3-6? The only "real changes" in the later patches
are the $$ -> $$$ thing and the NEWS entry as far as I can tell, and if
something should be in its own patch, that $$$ change is it. Oh right,
there's the re-computing change as well. But I don't think it's worth it,
just collapse it. The only sane way to look at this series is to diff
from 6/6 to the branch-point, which should tell you something. But I
don't feel strongly about it, it's just what I would have done. If 2/6
is a separate patch is not important to me either, so I would be ok with
you collapsing the whole series into a single co-authored patch. But then
again, I don't really care, and if you get tired from just thinking about
rewriting the commit message I can fully understand that position.

Another thing is that your new NEWS item is a bit awkward, and its
single sentence is simply too long and winding IMHO. The * heading
also needs an update.

>From your 5/6:

* Current directory in makefile fragments:

  - The special Automake-time substitutions '{RELDIR}' and '{CANON_RELDIR}'
    (and their abbreviated versions, '{D}' and '{C}' respectively) can now
    be used in an included Makefile fragment to indicate respectively the
    relative directory of that fragment and its canonicalized version,
    relative to the including Makefile:

My suggestion:

* Relative directory in Makefile fragments:

  - The special Automake-time substitutions '{RELDIR}' and '{CANON_RELDIR}'
    (and their abbreviated versions, '{D}' and '{C}' respectively) can now
    be used in an included Makefile fragment.  They are substituted with
    the relative directory of the included fragment, or its canonicalized
    version, compared to the top level including Makefile:

Cheers,
Peter

PS. You introduced the curdir naming, I had reldir in my original patch. :-)


Reply via email to