Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken commit history display if there's a visible merge commit #44

Open
2 tasks
taneliang opened this issue Aug 15, 2020 · 0 comments
Open
2 tasks

Broken commit history display if there's a visible merge commit #44

taneliang opened this issue Aug 15, 2020 · 0 comments
Labels
bug Something isn't working domain: frontend importance: 1 - must have Must be implemented for the product to be complete urgency: 2 - we can wait work type: investigation A task that primarily involves digging into a particular problem
Milestone

Comments

@taneliang
Copy link
Owner

taneliang commented Aug 15, 2020

If there's a merge commit above the earliest interesting commit, our frontend vomits blood.

Steps to repro

  1. Clone repro repo: https://github.com/24r/stack-attack-issue-44-repro. These are the branches:
    • main: the main branch, which includes a merge commit
    • eic: a branch pointing to the earliest interesting commit. This branch exists so that the interesting commit graph will contain a merge commit. This also means that if you delete this branch, Stack Attack will display the commit graph correctly.
    • c: just a random feature branch
  2. See Git commit graph:
    image
  3. Launch Stack Attack
  4. See the commit graph. The merge commit and everything above it are duplicated:
    image

Steps to resolve

  • Confirm that GSC is producing the commit graph correctly (it probably is). If it is, this bug is in the frontend
  • Decide how we want to solve this bug. Displaying a proper commit graph like Git may be too difficult as we'll need to implement a proper topographical sort of the commit history. We could try to find another way to display merge commits, e.g. maybe only showing merge commits when hasFork in displayCommitsForSubgraphRootedAtCommit is false.
@taneliang taneliang added the bug Something isn't working label Aug 15, 2020
@taneliang taneliang added this to the Launch milestone Aug 15, 2020
@taneliang taneliang added importance: 1 - must have Must be implemented for the product to be complete urgency: 2 - we can wait domain: SourceControl work type: investigation A task that primarily involves digging into a particular problem labels Aug 15, 2020
@taneliang taneliang changed the title Broken earliestInterestingCommit detection if there's a merge commit in the commit history Broken earliestInterestingCommit detection if there's a merge commit in the interesting commit history Aug 16, 2020
@taneliang taneliang changed the title Broken earliestInterestingCommit detection if there's a merge commit in the interesting commit history Broken commit history display if there's a visible merge commit Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working domain: frontend importance: 1 - must have Must be implemented for the product to be complete urgency: 2 - we can wait work type: investigation A task that primarily involves digging into a particular problem
Projects
None yet
Development

No branches or pull requests

1 participant