[01/15] t6120-describe: correct test repo history graph in comment
diff mbox series

Message ID 20190919214712.7348-2-szeder.dev@gmail.com
State New
Headers show
Series
  • name-rev: eliminate recursion
Related show

Commit Message

SZEDER Gábor Sept. 19, 2019, 9:46 p.m. UTC
At the top of 't6120-describe.sh' an ASCII graph illustrates the
repository's history used in this test script.  This graph is a bit
misleading, because it swapped the second merge commit's first and
second parents.

When describing/naming a commit it does make a difference which parent
is the first and which is the second/Nth, so update this graph to
accurately represent that second merge.

While at it, move this history graph from the 'test_description'
variable to a regular comment.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/t6120-describe.sh | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Junio C Hamano Sept. 20, 2019, 9:47 p.m. UTC | #1
SZEDER Gábor <szeder.dev@gmail.com> writes:

> At the top of 't6120-describe.sh' an ASCII graph illustrates the
> repository's history used in this test script.  This graph is a bit
> misleading, because it swapped the second merge commit's first and
> second parents.

Hmm...

> +#       ,---o----o----o-----.
> +#      /   D,R   e           \
> +#  o--o-----o-------------o---o----x
> +#      \    B            /
> +#       `---o----o----o-'
> +#                A    c

What's the first parent of the merge between 'B' and 'c' in this
picture and how does the reader figure it out?  What about the same
question on the direct parent of 'x'?  Is it generally accepted that
a straight line denotes the first ancestry, or something?  I do not
offhand see between these two the new one is a clear improvement.

I do agree with the issue with illustrating topology, and it is an
issue worth addressing.  In the past when the order of parents
mattered, I experimented to find ways to depict them clearly,
without much success.  One of the things I tried was to label the
parents, like so:

> -                       B
> -        .--------------o---1o---2o----x
> -       /                   2    1
> - o----o----o----o----o----.    /
> -       \        A    c        /
> -        .------------o---o---o
> -                   D,R   e

but I did not find it very satisfactory.

In any case, since this step is about "improving" the illustration,
I'd like to see a clear improvement.  Perhaps an extra comment that
says "straight line is the first parent chain" next to the drawing
might qualify as such.

Thanks.
SZEDER Gábor Sept. 20, 2019, 10:29 p.m. UTC | #2
On Fri, Sep 20, 2019 at 02:47:38PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > At the top of 't6120-describe.sh' an ASCII graph illustrates the
> > repository's history used in this test script.  This graph is a bit
> > misleading, because it swapped the second merge commit's first and
> > second parents.
> 
> Hmm...
> 
> > +#       ,---o----o----o-----.
> > +#      /   D,R   e           \
> > +#  o--o-----o-------------o---o----x
> > +#      \    B            /
> > +#       `---o----o----o-'
> > +#                A    c
> 
> What's the first parent of the merge between 'B' and 'c' in this
> picture and how does the reader figure it out?  What about the same
> question on the direct parent of 'x'?  Is it generally accepted that
> a straight line denotes the first ancestry, or something?

I've always thought that the parents are numbered from top to bottom,
i.e. 'B' is the first parent of the first merge, and the unnamed
commit at the top is the first parent of the second merge.

Would it help if it were arranged like this:

  o---o-----o----o----o-------o----x
       \   D,R   e           /
        \---o-------------o-'
         \  B            /
          `-o----o----o-'
                 A    c

This is basically how 'git log --graph' would show them, except that
this is horizontal.
Junio C Hamano Sept. 28, 2019, 4:06 a.m. UTC | #3
SZEDER Gábor <szeder.dev@gmail.com> writes:

>> Hmm...
>> 
>> > +#       ,---o----o----o-----.
>> > +#      /   D,R   e           \
>> > +#  o--o-----o-------------o---o----x
>> > +#      \    B            /
>> > +#       `---o----o----o-'
>> > +#                A    c
>> 
>> What's the first parent of the merge between 'B' and 'c' in this
>> picture and how does the reader figure it out?  What about the same
>> question on the direct parent of 'x'?  Is it generally accepted that
>> a straight line denotes the first ancestry, or something?
>
> I've always thought that the parents are numbered from top to bottom,
> i.e. 'B' is the first parent of the first merge, and the unnamed
> commit at the top is the first parent of the second merge.
>
> Would it help if it were arranged like this:
>
>   o---o-----o----o----o-------o----x
>        \   D,R   e           /
>         \---o-------------o-'
>          \  B            /
>           `-o----o----o-'
>                  A    c
>
> This is basically how 'git log --graph' would show them, except that
> this is horizontal.

Either is fine as long as they come with your "for a merge, earlier
parents are drawn near the top of the page" rule clearly described
near it (without such comment, I do not think either is clear enough).

Thanks.

Patch
diff mbox series

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 2b883d8174..0bf7e0c8bc 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -1,15 +1,14 @@ 
 #!/bin/sh
 
-test_description='test describe
+test_description='test describe'
+
+#       ,---o----o----o-----.
+#      /   D,R   e           \
+#  o--o-----o-------------o---o----x
+#      \    B            /
+#       `---o----o----o-'
+#                A    c
 
-                       B
-        .--------------o----o----o----x
-       /                   /    /
- o----o----o----o----o----.    /
-       \        A    c        /
-        .------------o---o---o
-                   D,R   e
-'
 . ./test-lib.sh
 
 check_describe () {