Hi Junio,
On Fri, 20 Apr 2018, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > The proof, as the saying goes, lies in the pudding. So here is a
> > regression test that not only demonstrates what the option is supposed to
> > accomplish, but also demonstrates that it does accomplish it.
>
> The above spreads the misconception that the value of the test is
> "what I wrote works, look here". It is not. "Here is how this
> thing is supposed to work. You are free to improve it, but do not
> break the basic promises these tests outline" to protect the
> resulting system is.
I am but a lousy foreigner, but to me, basically we said the same.
> > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> > index c630aba657e..77ded6df653 100755
> > --- a/t/t6050-replace.sh
> > +++ b/t/t6050-replace.sh
> > @@ -444,4 +444,24 @@ test_expect_success GPG '--graft on a commit with a mergetag' '
> > git replace -d $HASH10
> > '
> >
> > +test_expect_success '--convert-graft-file' '
> > + : add and convert graft file &&
> > + printf "%s\n%s %s\n%s\n" \
> > + $(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \
> > + >.git/info/grafts &&
>
> I find the above much less readbale than something like
>
> {
> git rev-parse HEAD^^
> git rev-parse HEAD^ HEAD^^
> git rev-parse HEAD^2
> } >.git/info/grafts
Well, don't you know, that is how my first version looked. It failed,
though, as `git rev-parse HEAD^ HEAD^^` outputs two *lines*.
And the version with a here-doc and inlined `$(git rev-parse ...)` really
looks even uglier.
> because printf formatting string must be deciphered and then matched
> against the order and number of rev-parse arguments (and printf's
> ability to happily accept more args than the placeholders does not
> help in readablity---the reader needs to verify that the code is not
> doing anything overly clever exploiting that ability) before I can
> figure out who's parent of whom.
>
> Of course, it saves a few spawning of subprocesses; I am not sure if
> that is worth the loss of readability in this case, though.
I disagree that it is so horrible to read. If a regression occurs, you
will have the .git/info/grafts file as a reference anyway, so there is
little you need to decipher.
Ciao,
Dscho
