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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(git): allow custom merge commit ids #3361

Merged
merged 1 commit into from Aug 30, 2022
Merged

feat(git): allow custom merge commit ids #3361

merged 1 commit into from Aug 30, 2022

Conversation

aloisklink
Copy link
Member

@aloisklink aloisklink commented Aug 25, 2022

馃搼 Summary

Currently, merge commits can have a git tag, but they cannot have a custom git commit ID.

This commit allows modifying the default merge commit id.

Resolves #3356

Partially resolves issue #3238 (most of the discussion there is talking about merge commit IDs, but technically the issue is asking for all merge commits to have all the same options as base commits).

馃搹 Design Decisions

In order to view the custom merge commit ids, I've undone commit 3ccf027.

This means that all merge commits have their IDs displayed, including auto-generated commit IDs. Personally, I think this is okay, since it's how git works, but it does add a bit of clutter to the git graphs.

If you want, I could hide merge commits labels behind a config option gitGraphConfig.showMergeCommitLabel.

I've also made a commit (aloisklink@954bf9e), where I update some mermaid-examples diagrams in the docs, but they're not included in this PR, since the docs currently only use v9.1.5 of mermaid:

mermaid/docs/index.html

Lines 21 to 22 in cde3a7c

<script src="//cdn.jsdelivr.net/npm/mermaid@9.1.5/dist/mermaid.min.js"></script>
<!-- <script src="http://localhost:9000/mermaid.js"></script> -->

Example changes:

%%{init: { 'logLevel': 'debug', 'theme': 'forest' } }%%
      gitGraph
        commit
        branch hotfix
        checkout hotfix
        commit
        branch develop
        checkout develop
        commit id:"ash" tag:"abc"
        branch featureB
        checkout featureB
        commit type:HIGHLIGHT
        checkout main
        checkout hotfix
        commit type:NORMAL
        checkout develop
        commit type:REVERSE
        checkout featureB
        commit
        checkout main
        merge hotfix
        checkout featureB
        commit
        checkout develop
        branch featureA
        commit
        checkout develop
        %% ID added here
        merge hotfix id: "bca"
        checkout featureA
        commit
        checkout featureB
        commit
        checkout develop
        %% Tag added here (already working before)
        merge featureA tag: "v1.1.0-rc1"
        branch release
        checkout release
        commit
        checkout main
        commit
        checkout release
        merge main
        checkout develop
        merge release

image

馃搵 Tasks

Make sure you

  • 馃摉 have read the contribution guidelines
  • 馃捇 have added unit/e2e tests (if appropriate)
  • 馃敄 targeted develop branch

Currently, merge commits can have a git tag, but they cannot have a
custom git commit ID.

This commit allows modifying the default merge commit id.
It also displays all merge commits IDs, which undoes
3ccf027
Copy link
Collaborator

@ashishjain0512 ashishjain0512 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aloisklink

Thank you for your contribution and effort. We really appreciate it. We had already started working on root issue #3238 to enhance the merge commit to allow all commit-specific attributes, including "id," "tag" and "type".

However, We will be reuseing some parts of your PR. Hence we will merge it, especially the test cases and documentation.

@ashishjain0512 ashishjain0512 merged commit d7e0888 into mermaid-js:develop Aug 30, 2022
@aloisklink aloisklink deleted the feature/3356_git_custom_merge_commit_id branch August 30, 2022 18:33
@aloisklink
Copy link
Member Author

We had already started working on root issue #3238 to enhance the merge commit to allow all commit-specific attributes, including "id," "tag" and "type".

That's my fault for not looking at #3238 and seeing that you were assigned to it (I only spotted that issue after I had made my PR).
Still, I'm happy you at least managed to reuse some of my test cases/documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow tags and IDs on merge commits in gitgraph diagrams
3 participants