W dniu 20.09.2016 o 21:02, larsxschneider@xxxxxxxxx pisze: > From: Lars Schneider <larsxschneider@xxxxxxxxx> > > Use `test_config` to set the config, check that files are empty with > `test_must_be_empty`, compare files with `test_cmp`, and remove spaces > after ">" and "<". That's good. > > Please note that the "rot13" filter configured in "setup" keeps using > `git config` instead of `test_config` because subsequent tests might > depend on it. This is good information to have for doing review (which could include "post-mortem" review during bisect, so it should be in commit message proper). > > Reviewed-by: Stefan Beller <sbeller@xxxxxxxxxx> I have not reviewed this patch in detail, but it looks good. A bit of nitpicking below. > Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx> > --- > t/t0021-conversion.sh | 58 +++++++++++++++++++++++++-------------------------- > 1 file changed, 29 insertions(+), 29 deletions(-) > > diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh > index e799e59..dc50938 100755 > --- a/t/t0021-conversion.sh > +++ b/t/t0021-conversion.sh > @@ -38,8 +38,8 @@ script='s/^\$Id: \([0-9a-f]*\) \$/\1/p' > > test_expect_success check ' This patch is "while at it" already for this patch series, done I guess for new tests to both use modern style, and be consistent with the rest of test... ...that said, if you could modernize _naming_ of tests. The t0021 test is quite inconsistent, and uses: * standard short names, like 'setup', without quotes (once), which is I think all right * cryptic short names, like 'check', without quotes (once) * snake_case name, like 'expanded_in_repo', without quotes (once) > test_expect_success "filter: clean empty file" ' > test_expect_success "filter: smudge empty file" ' * double quoted names (twice, see above) * proper modern names, with single quotes (the rest), which is as almost all the rest should be using Best, -- Jakub Narębski