Diagram on "About pull request merges" is misleading and confusing

[IMSoP]

Code of Conduct

• [X] I have read and agree to the GitHub Docs project's Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges

What part(s) of the article would you like to see updated?

The first diagram, showing a non-fast-forward merge, is extremely misleading, because it appears to show commits as "living on" a branch, and somehow "moving between them" when they are merged.

Most glaringly, the last line appears to show commit D no longer having any ancestors, when in reality it will still trace history back to commit C, and this is a very important feature of git.

It is also confusing that the label "Main" appears twice, leaving the reader to guess that one is before and one after the merge.

Current diagram in ASCII art for comparison:

Main: A -> B -> C
Feature: A -> B -> C -> D -> E
Main: A -> B -> C ----> F
                    /
              D -> E

Suggested replacement:

Before: A -> B -> C <-(main)
                  \
                   +-> D -> E <-(feature)

After: A -> B -> C -----------> F <-(main)
                  \          /
                   +-> D -> E <-(feature)

The second diagram, showing a squash merge, could be updated to match (where ~~~~~ represents the triangle of arrows linking D + E to F on the existing diagram):

Before: A -> B -> C <-(main)
                  \
                   +-> D -> E <-(feature)

After: A -> B -> C ----> F    <-(main)
                  \    ~~~~~~
                   +-> D -> E <-(feature)

Additional information

No response

[welcome[bot]]

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

[cmwilson21]

@IMSoP Thanks so much for opening an issue! I'll triage this for the team to take a look :eyes:

[jhosman]

👋 @IMSoP - Thanks for pointing this out! I see what you mean about the potential for confusion with the current screenshots. We'll need to redesign these screenshots internally, so I'm going to close this issue out and open an internal issue. Thanks again for letting us know!