Thread (16 messages) 16 messages, 4 authors, 2020-08-03

Re: [PATCH 2/3] t4216: fix broken '&&'-chain

From: Jeff King <hidden>
Date: 2020-06-30 19:19:46

On Tue, Jun 30, 2020 at 03:12:31PM -0400, Taylor Blau wrote:
quoted
I think so, but my argument would be more along the lines of:

  - without "-f", "rm" will complain about a missing file, which is
    distracting noise in the test log

  - once "-f" is added in to suppress that, we might as well add the
    command to the &&-chain. That's our normal style, so readers don't
    have to wonder if it's important or not. Plus it would help avoid a
    broken chain if more commands are added at the beginning of the
    function.
I made the change for basically these reasons, but mostly to bring this
function into good style as with the rest of our test suite (there are a
handful of other minor nits that we could look at, such as some odd
spacing, etc.).

Whether or not all of this needs to go into the commit message... I
don't know. On the one hand, I think that your explanation here is
clearer than what I wrote in the commit message, but on the other hand,
I think that amending it again may be belaboring an otherwise simple
change.

If you feel strongly, though, I'm happy to send a revised patch.
I agree it's a pretty trivial patch, but I think if it's worth applying
at all, then it's worth justifying it appropriately.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help