-
Notifications
You must be signed in to change notification settings - Fork 28
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
Vindigo cumulus 3184 #1124
base: develop
Are you sure you want to change the base?
Vindigo cumulus 3184 #1124
Conversation
@@ -25,13 +25,14 @@ const ExecutionStatusGraph = ({ executionStatus }) => { | |||
const workflow = JSON.parse(stateMachine.definition); | |||
const events = getExecutionEvents(executionHistory); | |||
const graph = workflowToGraph(workflow); | |||
console.log('graph', graph); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a couple clarifying questions but I don't see an issue as long as tests pass.
package.json
Outdated
@@ -149,11 +149,10 @@ | |||
"compare-versions": "^4.1.2", | |||
"connected-react-router": "^6.9.2", | |||
"console-browserify": "^1.2.0", | |||
"core-js": "^3.20.1", | |||
"core-js": "^3.35.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this also due to the dagre update or a separate concern? Fine either way, just clarifying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change was to avoid conflict with develop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like a merge conflict? It's probably fine to update a package like this but what was the conflict?
package.json
Outdated
@@ -171,7 +170,7 @@ | |||
"path-browserify": "^1.0.1", | |||
"postcss": "^8.4.5", | |||
"process": "^0.11.10", | |||
"prop-types": "^15.8.0", | |||
"prop-types": "^15.8.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question. Was this a vulnerability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change was to avoid conflict with develop.
Changed git+ssh to git+https in package-lock file
boom audit error
Pulling in #1123
Summary: Summary of changes
Addresses CUMULUS-3184: Dependency d3-dag code integration to d3 graph
Changes
dagre-d3
withdagre-d3-es
to address thed3-color
vulnerabilityd3
tov7.2.0
to be compatible withdagre-d3-es
v7.0.10
PR Checklist