stop trying to use the ecmarkdown parse tree from the linting phase #393
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I just noticed that 80959f0 doesn't work when passing
--lint-spec, because that option moves the initial parse to an earlier phase and saves the result.This PR drops that attempt to save on work and instead unconditionally parses the html again. This doesn't materially affect runtime, since ecmarkdown parsing is a tiny fraction of overall runtime (I did three runs building the spec with
--lint-specbefore and after this change and got [15.899s, 15.902s, 15.474s] and [15.799s, 15.620s, 15.297s] respectively).We should maybe revisit this logic at some point, but this is fine for now.
I also updated the tests to run all the baselines with
--lint-specto ensure enabling linting doesn't affect the build output so this can't happen again. This adds 2 seconds to the test suite (on my machine), for about 8s -> 10s, which is a price I'm willing to pay.