feat(pass-style): Extract passStyleOf and friends from marshal into the new pass-style package #1439
Conversation
erights
changed the title
erights
force-pushed
the
markm-extract-pass-style
branch
from
0c132f9
to
cec65d4
Compare
kriskowal
force-pushed
the
kris-scaffold-pass-style
branch
2 times, most recently
from
bc0f2e5
to
2fcf1e7
Compare
erights
force-pushed
the
markm-extract-pass-style
branch
from
a9d3455
to
95ea0b6
Compare
| @@ -112,6 +113,9 @@ const makeNoIndenter = () => { | |||
| }; | |||
|
|
|||
| const identPattern = /^[a-zA-Z]\w*$/; | |||
| harden(identPattern); | |||
| const AtAtPrefixPattern = /^@@(.*)$/; | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Let's leave this conversation unresolved, so that anyone visiting this PR sees this ;)
erights
force-pushed
the
markm-extract-pass-style
branch
from
4120baf
to
9f05227
Compare
|
After @kriskowal merged #1438 , I got this PR working in all ways except for lint errors, as represented by the first commit. Of course I want to be green under CI before merging. But for many of the type errors reported by lint, I could not figure them out, so I silenced them instead, as shown in the second commit 1046a4e . Advice? Do you mind if I merge this to unblock @kriskowal, while filing an issue to undo the damage of that second commit? @turadg @kriskowal , I hit the "Re-request review" button on both of you because of this question. Thanks. |
|
Oh damn. We are indeed using The same suppression strategy seems to work, so I added it to the new second commit at bf9fa86 |
erights
force-pushed
the
markm-extract-pass-style
branch
from
1046a4e
to
01de07b
Compare
erights
force-pushed
the
markm-extract-pass-style
branch
from
01de07b
to
bf9fa86
Compare
There was a problem hiding this comment.
I hit the "Re-request review" button on both of you because of this question.
The type suppressions seem appropriate given the scope of this PR. The comments leave a good trail for investigation. It's not totally surprising that changing the dependency graph would yield different resolution of the ambient types.
I would like to see the import/order suppressions removed or justified with a comment.
For that reason and because I'm not fully acquainted with this code or the PR motivation I'll make this just a Comment and not Approve.
Staged on #1438
Fixes: #1441
Fixes: #1442
Fixes: #1443.
To test #1438 , I moved the passStyleOf level of abstraction from marshal into this new pass-style scaffold. But I did it as a separate PR so we can keep #1438 as a record of what the scaffold looks like.
This test has so far revealed a linting problem, but otherwise seems to work. As #1438 is fixed, I'll continually rebase this PR on it to test the newer candidates.
I started out planning to only do a little of the passStyleOf migration, just to test #1438 . But once I started I couldn't stop. Seems to be close to complete.