-
Notifications
You must be signed in to change notification settings - Fork 46
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
front: avoid unecessary path finding call in stdcm #10288
Conversation
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.
Why are the pathSteps
updated when touching the calendar in the first place?
Shouldn't we avoid this update instead? Otherwise we'd need to carry this workaround each time pathStep
is in a useEffect()
/useMemo()
dependency.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10288 +/- ##
==========================================
+ Coverage 81.53% 81.56% +0.02%
==========================================
Files 1059 1062 +3
Lines 104508 104968 +460
Branches 722 722
==========================================
+ Hits 85213 85617 +404
- Misses 19254 19310 +56
Partials 41 41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Temporal inputs are stored in the corresponding step. Steps defines the whole path with its constraints, which makes sense. Ex: In our case we need a derivative data because we just want to find a path (not a schedule). |
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.
Eh, sorry, I thought we had separate arrays for path
and schedule
, but everything is in a single StdcmPathStep
type.
I don't see a better way to fix this bug!
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.
Lgtm and tested
fix #10144 Adding a state for pathStepsLocations and only changes its value when there is a deep diff Signed-off-by: Benoit Simard <contact@bsimard.com>
fix #10144
Adding a state for
pathStepsLocations
and only changes its value when there is a deep diff