Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGet rid of getPhases #272
Get rid of getPhases #272
Conversation
Current coverage is 90.33%@@ master #272 diff @@
==========================================
Files 51 51
Lines 6926 6904 -22
Methods 0 0
Messages 0 0
Branches 1332 1329 -3
==========================================
- Hits 6264 6237 -27
- Misses 501 506 +5
Partials 161 161
|
|
@gsnedders Is this important? There's no explanatory issue, so I'm not sure what the value is here and whether we should spend time on it for 1.0 or not. |
|
@willkg Reduction in complexity and potentially performance gains by reducing indirection. |
|
Ok. I'm going to push this off until after 1.0, then. |
Codecov Report
@@ Coverage Diff @@
## master #272 +/- ##
==========================================
- Coverage 91.07% 91.03% -0.04%
==========================================
Files 50 50
Lines 7044 7016 -28
Branches 1341 1337 -4
==========================================
- Hits 6415 6387 -28
Misses 475 475
Partials 154 154
Continue to review full report at Codecov.
|
This added a fair bit of complexity, and notable made the Phase classes dynamically generated. However, by doing this, we no longer include "process the token using the rules for" phases in the debug log.
With this we no longer include "process the token using the rules for" phases in the debug log.
This also needs perf review given it touches
mainLoop.(If you want to view the diff, append
?w=1to the URL to ignore whitespace, otherwise the reindenting of the phases just dominates.)