Remove unused Redux selectors #1284
Conversation
mszabo-wikia
requested review from
everett980,
rubenvp8510 and
tiffon
as code owners
Some of the inconsistency of coverage results is due to randomly generated test data in trace.test.js. While investigating, however, I noticed that most of the Redux selectors in this directory were only referenced by each other and by tests - it seems they were used at some point (they're in use in the initial commit) but were progressively removed by refactorings. Since selectors only serve to encapsulate and reuse logic to lookup data from the Redux store[1], they are not useful when not imported. As such, take this opportunity to remove the dead code. --- [1] https://redux.js.org/usage/deriving-data-selectors Signed-off-by: Máté Szabó <mszabo@fandom.com>
mszabo-wikia
force-pushed
the
remove-unused-selectors
branch
from
dcdde4f
to
5d2cd36
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #1284 +/- ##
==========================================
+ Coverage 95.44% 95.50% +0.05%
==========================================
Files 244 243 -1
Lines 7753 7627 -126
Branches 2016 2002 -14
==========================================
- Hits 7400 7284 -116
+ Misses 353 343 -10
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
The other file where coverage changes randomly between local test runs is https://github.com/jaegertracing/jaeger-ui/blob/main/packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx. Not yet sure why. |
Any idea what would compel people to change away from using selectors? Do you think it's just general unawareness that they exist? |
|
Looks like #68 was the PR that removed most usages, it shifted part of the state management into duck.js (duck.tsx). There isn't much context specifically why though. |
## Which problem is this PR solving? - Contributes towards making coverage reports more consistent ## Short description of the changes Some of the inconsistency of coverage results is due to randomly generated test data in trace.test.js. While investigating, however, I noticed that most of the Redux selectors in this directory were only referenced by each other and by tests - it seems they were used at some point (they're in use in the initial commit) but were progressively removed by refactorings. Since selectors only serve to encapsulate and reuse logic to lookup data from the Redux store[1], they are not useful when not imported. As such, take this opportunity to remove the dead code. --- [1] https://redux.js.org/usage/deriving-data-selectors Signed-off-by: Máté Szabó <mszabo@fandom.com>
Which problem is this PR solving?
Short description of the changes
Some of the inconsistency of coverage results is due to randomly generated test data in trace.test.js. While investigating, however, I noticed that most of the Redux selectors in this directory were only referenced by each other and by tests - it seems they were used at some point (they're in use in the initial commit) but were progressively removed by refactorings. Since selectors only serve to encapsulate and reuse logic to lookup data from the Redux store[1], they are not useful when not imported. As such, take this opportunity to remove the dead code.
[1] https://redux.js.org/usage/deriving-data-selectors