On Mon, Sep 19, 2016 at 02:16:23PM -0700, Junio C Hamano wrote: > Kevin Daudt <me@xxxxxxxxx> writes: > > > Many tests need to store data in a file, and repeat the same pattern to > > refer to that path: > > > > "$TEST_DATA"/t5100/ > > That obviously is a typo of > > "$TEST_DIRECTORY/t5100" > > It is a good change, even though I would have chosen a name > that is a bit more descriptive than "$DATA". The name "$DATA" was my suggestion. I was shooting for something short since this is used a lot and is really a script-local variable (I'd have kept it lowercase to indicate that, but maybe that is just me). Something like "$root" would also work. I dunno. > > - test_cmp "$TEST_DIRECTORY"/t5100/msg$mo msg$mo && > > - test_cmp "$TEST_DIRECTORY"/t5100/patch$mo patch$mo && > > - test_cmp "$TEST_DIRECTORY"/t5100/info$mo info$mo > > + test_cmp "$DATA"/msg$mo msg$mo && > > + test_cmp "$DATA"/patch$mo patch$mo && > > + test_cmp "$DATA"/info$mo info$mo > > make me wonder why we don't quote the whole thing, i.e. > > test_cmp "$TEST_DATA/info$mo" "info$mo" > > as leaving $mo part unquoted forces reader to wonder if it is our > deliberate attempt to allow shell $IFS in $mo and have the argument > split when that happens, which can be avoided if we quoted more > explicitly. > > Perhaps we'd leave that as a low-hanging fruit for future people. Yeah, I agree that quoting the whole thing makes it more obvious (though I guess quoting the second info$mo does add two characters). -Peff