Junio C Hamano <gitster@xxxxxxxxx> writes: >> +--no-hash:: >> + Output an all-zero hash in each patch's From header instead >> + of the hash of the commit. >> + > > Two (big) problems with the option name. > > - "--no-something" would mislead people to think you are removing > something, not replacing it with something else. This option > does the latter (i.e. the first line of your output still has > 40-hex; it's just it no longer has a useful 40-hex). > > - There are many places we use hexadecimal strings in format-patch > output and you are not removing or replacing all of them, only > the commit object name on the fake "From " line. Saying "hash" > would mislead readers. I am not good at bikeshedding, but you used 'zero_commit' elsewhere in the code. I think that would be a much better name--perhaps use that consistently throughout, as the local variable in options[] array and end-user facing option name? > >> +test_expect_success 'format-patch --no-hash' ' >> + git format-patch --no-hash --stdout v2..v1 >patch2 && >> + cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) && > > Don't test "any number of '0'"; test 40 '0's. This is because the > line format was designed to be usable by things like /etc/magic to > detect format-patch output, and we want to notice if/when we break > that aspect of our output format. > >> + test $cnt = 3 >> +' >> + >> test_done > > Thanks. -- 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