automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] dist-xz, dist-bzip2: don't hard-code -9: honor envvar settin


From: Stefano Lattarini
Subject: Re: [PATCH] dist-xz, dist-bzip2: don't hard-code -9: honor envvar settings
Date: Sat, 10 Dec 2011 12:52:55 +0100
User-agent: KMail/1.13.7 (Linux/2.6.30-2-686; KDE/4.6.5; i686; ; )

On Saturday 10 December 2011, Jim Meyering wrote:
> I have not taken the time to write a commit hook to warn me when
> a git log fails to match  the corresponding ChangeLog delta.
> It doesn't seem worthwhile.
>
Absolutely agreed; especially because, as long as the ChangeLog
is version-controlled, it can be fixed with later patches ...

> ...
> > You have forgotten to update the recipes for `dist' and `dist-all':
> >
> >   $ grep -e -9 lib/am/distdir.am
> >           tardir=$(distdir) && $(am__tar) | BZIP2=$${BZIP2--9} bzip2 -c 
> > >$(distdir).tar.bz2
> >           tardir=$(distdir) && $(am__tar) | lzma -9 -c >$(distdir).tar.lzma
> >   ?BZIP2? tardir=$(distdir) && $(am__tar) | bzip2 -9 -c >$(distdir).tar.bz2
> >   ?LZMA?  tardir=$(distdir) && $(am__tar) | lzma -9 -c >$(distdir).tar.lzma
> >   ?XZ?    tardir=$(distdir) && $(am__tar) | xz -9 -c >$(distdir).tar.xz
> 
> Good catch.
> 
> > I've fixed this "the dumb way", i.e., by copying & pasting.  Extra
> > points will go to anyone which comes up with a refactoring patch that
> > removes this annoying code duplication.
> 
> That looks fine for now.
>
Also, I've noticed that such a refactoring is already present in master.

> > And a final nit: you have forgotten to run ./boostrap to regenerate
> > Automake's own Makefile.in (sorry, theis is still version-controlled
> > for now!).  I've fixed that as well.
> >
> > Attached is what I've pushed to maint (and merged into branch-1.11).
> >
> > Thanks,
> >   Stefano

> > From c8e01d581a7e7c2445a139def46939a547951746 Mon Sep 17 00:00:00 2001
> > Message-Id: <address@hidden>
> > From: Jim Meyering <address@hidden>
> > Date: Fri, 9 Dec 2011 23:17:18 +0100
> > Subject: [PATCH] dist-xz, dist-bzip2: don't hard-code -9, honor envvar 
> > settings
>
> [SNIP]
>
> > This is inspired to commit v1.11-389-g6da46f3, with additional
> 
> s/inspired to/inspired by/
>
Ouch.  But I have already pushed, so we can't change the git commit
message without distrupting the history; this typo will have to
live on.  Annoying, but no biggie.

> And in commit logs I like to use the 8-hex-digit SHA1
> representation (perhaps in addition to something like you've
> provided) because gitk automatically detects that and renders
> it as a clickable link.
>
But reading from Automake's HACKING file:

 * When referring to older commits, use 'git describe' output as
   pointer.

Do you think your use case would make it worth to change this policy
and introducing another (admittedly small) inconsistency in the git
commit messages from now on?  I have no strong feelings, but I'd
rather avoid another "policy churn" (albeit small) if the gain would
be only marginal.

> I admit that in a non-interactive rendering of the log,
> your longer form is much more useful.
>
Also, note that at least git, qgit and gitk seems to understand the
output of git-describe as well *when given as an argument on the
command line*.

Maybe you could file a feature-request to gitk and/or qgit so that
they can recognize and highlight `git describe' outputs as well as
eight-hexdigit SHA1.  WDYT?

Thanks,
  Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]