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

Make gitGraph commit IDs out of hexadecimal chars #1496

Merged
merged 1 commit into from Jul 12, 2020

Conversation

quulah
Copy link
Contributor

@quulah quulah commented Jun 24, 2020

馃搼 Summary

This PR makes the commit IDs created by gitGraph to look a bit more like actual short hashes from Git.

馃搹 Design Decisions

I just set the characters back to what they were prior to #1408 changes.

馃搵 Tasks

Make sure you

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

@quulah
Copy link
Contributor Author

quulah commented Jun 24, 2020

I saw the branch naming convention only after making the change. Hopefully that's not too big of an issue. :)

@knsv knsv requested a review from ashishjain0512 July 5, 2020 13:12
@knsv
Copy link
Collaborator

knsv commented Jul 12, 2020

@quulah Hi, thinks for this fix which is more correct than the current implementation.

I did have one concern which is why this is not already merged and that is backwards compatability. We are always very careful when it comes to backwards compatability of authored diagrams. It is one thing to change that API so that someone that upgrades mermaid in a plugin or directly on a sites needs to make a change or two for the new version. That is perhaps unfortunate but in principal ok, it is a very different story when it comes to authored diagrams that endusers have composed.

Here that is not the case as this id is generated by mermaid and now it will be a little more realistic.

@knsv knsv merged commit 2bdc229 into mermaid-js:develop Jul 12, 2020
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.

None yet

2 participants