On 2023-01-14 22:34, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Junio C Hamano <gitster@xxxxxxxxx> writes: >> >>> "Strawbridge, Michael" <Michael.Strawbridge@xxxxxxx> writes: >>> >>>> +test_expect_success $PREREQ "--validate hook supports header argument" ' >>>> + test_when_finished "rm my-hooks.ran" && >>>> + write_script my-hooks/sendemail-validate <<-\EOF && >>>> + filesize=$(stat -c%s "$2") >>> >>> That "stat -c" is a GNU-ism, I think. macOS CI jobs at GitHub do >>> not seem to like it. >>> >>>> + if [ "$filesize" != "0" ]; then >>> >>> Also, please see Documentation/CodingGuidelines to learn the subset >>> of shell script syntax and style we adopted for this project. > > I'll tentatively queue this as a fix-up on top of the topic, but is > this testing the right thing? Should we inspect "$2" and verify > that it gives us what we expect, not just it being non-empty? Hi Junio, Thanks for reviewing this patch set. We're generally not interested in "what else" is in the SMTP envelope and headers. The extension this patch set provides is that if a hook-writer is interested in some SMTP header, or the contents of that header, then there is a way to provide the SMTP envelope and thus check the headers. Currently, $1, is identical to git-format-patch's output, (for which there are other hooks to check that output.) This was a bit disappointing, as it is a git-send-email hook after all, and we're interested in the "email" part of this Git command and hook. The idea is that hook writers would merely be grepping for a particular header they're interested in--it could even be a custom header, "X-something" for instance, and if present, they'll check the contents of that header and validate the patch, or perform some other action. So, checking that the SMTP envelope and headers, $2, is not empty suffices for what this patch set implements. We leave it up to the hook writers to inspect the SMTP envelope and headers for their particular hook purpose. -- Regards, Luben
