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