Fix code duplication in ExplorationPlayerStateService #6000
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #6000 +/- ##
=======================================
Coverage 45.3% 45.3%
=======================================
Files 520 520
Lines 30667 30667
Branches 4597 4597
=======================================
Hits 13893 13893
Misses 16774 16774
Continue to review full report at Codecov.
|
brianrodri
changed the title
brianrodri
changed the title
brianrodri
requested review from
BenHenning and
nithusha21
and removed request for
aks681
There was a problem hiding this comment.
Overall LGTM. Thanks! One question. Do we not prefer variables/function names prefixed with _? I think we previously used it to refer to private variables/functions. If not, would you like to have a lint check that prevents this?
Also, @BenHenning what's your opinion on names prefixed with _?
Also did you revert the issues -> improvements change as it was done in the other PR? Just confirming!
|
I've double-checked our style guide, and I've found:
And yeah, I accidentally committed the issues -> improvements change to this PR, but it was meant to be part of the other. Going to go ahead and merge now, thanks! |
|
Ah thanks for checking! Would you like to have the linting checks include this? If so, I would suggest filing an issue for the same! |
Code Health improvements to ExplorationPlayerStateService. Without them, it is error-prone to add dependencies or fixes to the service since there are so many branches to consider. This tries to consolidate the pieces that belong close together.