-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
refactor(ChartDataCommand): into two separate commands #17425
refactor(ChartDataCommand): into two separate commands #17425
Conversation
4dde8ae
to
25da471
Compare
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.
Separation looks clean !
Codecov Report
@@ Coverage Diff @@
## master #17425 +/- ##
==========================================
- Coverage 76.97% 76.97% -0.01%
==========================================
Files 1041 1041
Lines 56069 56068 -1
Branches 7742 7742
==========================================
- Hits 43157 43156 -1
Misses 12654 12654
Partials 258 258
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Background
When we have worked on #16991 we wanted to test the new functionalities in concrete and accurate unittest.
All chartData flows and its components are too couple to superset so it is impossible to create unittests.
The flows are not testable and so many components do not meet the very important principle SRP and the code became so dirty
So I've started to refactor it (#17344 ) but many changes were added and it was hard to review so I decided to split those changes into small PRs so will be easier to follow
This is the fifth PR in a sequence of PRs to meet these
The next PR is #17461
PR description
Note - The pr consists of the #17407 commit until it will be merged then I'll recreate the branch from master
Test plans
There are no logic changes so new tests are not required
Previous PRs