Re: [PATCH v2 09/11] t/t3437: cleanup the 'setup' test and use named commits in the tests

Charvi Mendiratta [off-list ref] writes:

Remove unnecessary curly braces and use the named commits in the
tests so that they will still refer to the same commit if the setup
gets changed in the future whereas 'branch~2' will change which commit
it points to.

Doing two things in the same commit?  I think ${EMPTY} thing is a
general style clean-up, while tagging is a bit more meaningful
change to make it easier to understand tests and is a change at a
more conceptual level.  The ${EMPTY} change would be better done at
the same time when the here document was cleaned up in [v2 05/11],
I would think.

Thanks.

On Tue, 9 Feb 2021 at 03:11, Junio C Hamano [off-list ref] wrote:

Charvi Mendiratta [off-list ref] writes:

quoted

Remove unnecessary curly braces and use the named commits in the
tests so that they will still refer to the same commit if the setup
gets changed in the future whereas 'branch~2' will change which commit
it points to.

Doing two things in the same commit?  I think ${EMPTY} thing is a
general style clean-up, while tagging is a bit more meaningful
change to make it easier to understand tests and is a change at a
more conceptual level.  The ${EMPTY} change would be better done at
the same time when the here document was cleaned up in [v2 05/11],
I would think.

Okay, will move it to the other patch.

Thanks and Regards,
Charvi