Get rid of getPhases#272
Conversation
Sorry, something went wrong.
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
|
Sorry, something went wrong.
gsnedders
force-pushed
the
constant_phases
branch
from
01cc4a8 to
a120557
Compare
July 13, 2016 22:47
|
@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. |
Sorry, something went wrong.
|
@willkg Reduction in complexity and potentially performance gains by reducing indirection. |
Sorry, something went wrong.
|
Ok. I'm going to push this off until after 1.0, then. |
Sorry, something went wrong.
gsnedders
force-pushed
the
constant_phases
branch
from
a120557 to
d764def
Compare
June 23, 2020 03:05
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.
|
Sorry, something went wrong.
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.
gsnedders
force-pushed
the
constant_phases
branch
from
db7310d to
8cff6aa
Compare
September 27, 2020 20:44
jayaddison
left a comment
jayaddison
left a comment
There was a problem hiding this comment.
NB: I've no write or commit access here, but to me these changes look good.
From local checkout & review:
- Tests pass
- Performance impact (for
parserbenchmarks, since that's what this code affects): neutral (cpython 3.9.1)
Although there doesn't seem to be a performance improvement from this, in my opinion it does work towards making the code simpler and opens up future refactoring and cleanup work that is likely to improve performance in more significant ways.
Sorry, something went wrong.
Reviewers
1 more reviewer
6 participants
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.)