-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Release/10.7.0 #5188
Merged
Merged
Release/10.7.0 #5188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed argument names from commit1 and commit2 to commitA and commitB respectively to prevent confusion with seq number values. Replaced Array filter method with array some method so that as soon as one overlap is found, function is finished. Used Object.entries instead of Object.keys to reduce number of variables needed and make function easier to read.
On previous rewrite, I had created new functions within the overlapping functions but these were being recreated on each iteration of Object.some(). I moved them outside this for clarity and so they're not recreated each iteration.
The function also now does an additional check to see if source branch in overlap check is on main. As we're no longer purely checking for an overlap and the only use of this function is to reroute the arrows to avoid running over commits, this more literal name should be clearer.
Checking if branch was same as main turned out to be redundant for now, since there don't seem to be any cases where routing curves into main. This fixes issue found in review by @nirname and avoids a situation where branching from the same commit results in unnecessary rerouting.
Fix for error spotted by @mathbraga
Followed practice of other tests so that commit IDs are stabilised (i.e., not randomly generated) and therefore don't show repeatedly in Cypress diff output screenshots
Hypothesised that working out which branch needed checking for overlapping commits might be missing, so added that as a nested ternary and passed result as new argument to rerouting check. If commits are found on the branch which will be getting the curve (whichever branch is lower or more to the right of main than the other, for now), then the arrow will be rerouted. I may refactor in a follow-up commit and I think there's scope to simplify the logic but this is a test for now.
My focus earlier on had been on relationship to `main` branch so this is to ensure that we have some tests that cover relationship between a pair of branches that doesn't include `main`.
Based on review by @nirname. I had originally been trying to minimise number of new arguments being passed to rerouting check but as the branch curve check is not used elsewhere and is part of the same rerouting check it makes sense for them to be together. Position information now passed to rerouting fn instead.
Should stop randomised commit IDs from showing arbitrary differences between test runs.
Wanted to avoid repetition given that the originally nested ternaries had the same structure
Pre-commit lint hook had made the use of ternaries harder to read than I'd originally intended so I introduced an additional variable which explains what is being checked and keeps ternaries from becoming obscured.
DOCS: update Flowchart page
* develop: update verbiage update verbiage
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5188 +/- ##
===========================================
- Coverage 76.60% 62.94% -13.67%
===========================================
Files 164 108 -56
Lines 13820 10398 -3422
Branches 693 519 -174
===========================================
- Hits 10587 6545 -4042
- Misses 3077 3740 +663
+ Partials 156 113 -43
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…vity tools using Mermaid.)
Update integrations-community.md (Add Codemia to the list of productivity tools using Mermaid)
4 tasks
This was referenced Jan 15, 2024
…graph-title-margin-config-option Revert 5041 feature/4935 subgraph title margin config option
…-order-sequence-participant Revert "fix: render the participants in same order as they are created"
…/mermaid into release/10.7.0 * 'master' of github.com:mermaid-js/mermaid: add inadvertent tracking removal update announcement bar * 'release/10.7.0' of github.com:mermaid-js/mermaid:
…10.7.0 * 'develop' of github.com:mermaid-js/mermaid: Revert "fix: render the participants in same order as they are created" Revert "Feature/4935 subgraph title margin config option" Update integrations-community.md (Add Codemia to the list of productivity tools using Mermaid.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.