qemu-trivial
[Top][All Lists]
Advanced

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

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Use $(..) instead of


From: Maciej W. Rozycki
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Use $(..) instead of deprecated `..`
Date: Wed, 8 Jun 2016 15:38:01 +0100
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)

On Wed, 8 Jun 2016, Eric Blake wrote:

> >  Unlike `..` the $(..) Bourne shell construct is not fully portable, some 
> > implementations do not recognise it.
> 
> All POSIX implementations support it. The only shell that doesn't is
> from Solaris /bin/sh, and that pre-dates modern OpenSolaris which has
> (finally) modernized their shell.  And I don't think we have anyone
> actively trying to build qemu for Solaris (we have bug reports for
> mingw, BSD, and Mac OS, but I haven't seen anyone complaining about a
> failed Solaris build).

 The IRIX shell doesn't support it either I believe, as I suppose some 
other older systems.  We may or may not care about them (we might as well 
kindly ask people to install bash or suchlike), but I think it makes sense 
to make it explicit.

> >  NB given the above, and especially because of the introduced functional 
> > regression mentioned, I don't think this change qualifies as trivial.
> 
> I agree that calling this trivial may be a stretch, but I see no problem
> with the patch itself if the commit message is beefed up to provide more
> justification than just "silence a lint-like tool".

 Agreed, I think it's worth the little extra time needed to write it and 
have a commit description explanatory enough for anyone to understand the 
motivation.  Having one often turns a questionable change into an obvious 
one, not to mention that someone might scratch their head staring at a 
commit years on from now as they try to understand what the author 
actually meant to do.

 I find myself doing this frequently when discovering bugs introduced to 
software 10-15 years ago with commits having little if any description (as 
per the standards of that time), trying to figure out how to fix the bug
while not breaking the original intent of the change.

  Maciej



reply via email to

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