Pete Wyckoff wrote: > This addresses all of Jonathan's comments, in particular: Nice. Thanks much. I only have a few small worries left: [...] > +++ b/t/t9300-fast-import.sh > @@ -2635,4 +2635,280 @@ test_expect_success \ [...] > +test_expect_success 'S: filemodify with garbage after sha1 must fail' ' > + sha1=$(grep -w :103 marks | cut -d\ -f2) && "grep -w" isn't used elsewhere in the testsuite. Is it portable? [...] > +# inline is misspelled; fast-import thinks it is some unknown dataref > +# and complains "Invalid SHA1" > +test_expect_success 'S: notemodify with garbage after inline dataref must fail' ' > + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err && > + commit refs/heads/S > + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE > + data <<COMMIT > + commit S note dataref inline > + COMMIT > + N inlineX :2 > + data <<BLOB > + note blob > + BLOB > + EOF > + cat err && > + test_i18ngrep "nvalid SHA1" err > +' If I understood the discussion before correctly, this error message is suboptimal and something like "invalid dataref" would be a little clearer, right? That's orthogonal to what this patch is about so I'm not suggesting changing it. But shouldn't the test just check that fast-import fails without committing to any particular message? Cheers, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html