On Mon, May 27, 2019 at 4:27 PM Elijah Newren <newren@xxxxxxxxx> wrote: > That sounds good...though it's taking my short patch and just about > amounts to completely rewriting it. Would you like to take it over > including authorship, and just add either a "Original-patch-by:" or > "Based-on-patch-by:" for me in the commit message (these two tags > appear to be the two most common attribution mechanism used in > git.git's history when someone does this)? Sure. Co-authored-by is an option too. I'll turn all of this in 3 patches: improve background_import, your patch, the rest of mine. > > > + cat >input <<-INPUT_END && > > > + feature done > > > + commit refs/heads/V3 > > > + mark :3 > > > + committer Me My <self@xxxxxxx> 1234567890 +0123 > > > > You likely want to use: > > committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE > > I see other tests in that testsuite using this, and using it here > certainly wouldn't hurt; I'm not opposed to it. But I'm > curious...other than "other tests in the same testcase use it a lot" I > don't see why the choice of committer name/email/date matters at all. > Is there an actual reason for this that I just missed? There is no direct benefit here except consistency. These vars are set the same on every test run so that the SHA1 of generated patches is the same as in a previous run (see also test_tick() to increment the date by one minute). So in general this makes it easier to debug tests as hashes can be compared between runs. But your hand-picked values do give you that property. However, as normal git commands use the vars automatically, by using them here, fast-import commands and direct git commands produce the same hashes if they're doing the same work. That's the main benefit of using them here: while the test does not currently rely on that property, maybe it will (or someone debugging down the road will expect that).
