-
Notifications
You must be signed in to change notification settings - Fork 818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(workbox-build): make source map filename consistent with injected source file #3234
base: v7
Are you sure you want to change the base?
Conversation
@jeffposnick would it be possible to get your review here? I just don't want to build a pipeline using this patch if you feel it's a no go. |
CC: @tropicadri and @petele for guidance. |
At this point I'm wondering if |
049d672
to
bf6c75a
Compare
bf6c75a
to
bc24e36
Compare
@tomayac any chance this could get looked at soon? It's not exactly a very taxing review. |
At first sight this LGTM, but the tests fail. Could you look if it's related to this change? Thanks! |
I know they passed locally when I first created the PR but I'll take a quick look. |
The same 8 tests fail on the v7 branch, so they have nothing to do with these changes. |
Thanks for checking! @tropicadri, over to you for a final look. I LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, approval.
Tests were manually verified to fail for unrelated reasons.
@tropicadri are you able to review and release this change? I'd like to avoid creating a patch if I can. |
Fixes #3233 per comments on #2239
swDest
with.map
appendedsourceMappingURL
comment with the new filename