Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implementation? #6

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Implementation? #6

wants to merge 3 commits into from

Conversation

phated
Copy link
Contributor

@phated phated commented Apr 1, 2017

Opening this for @nmccready and @floridoo to review. This module should be able to replace sourceMappingURL, sourceMappingURLPrefix and addComment (comment will always be added but can be removed with the comment.remove() stream) options in gulp-sourcemaps.

@phated
Copy link
Contributor Author

phated commented Apr 1, 2017

Forgot to enable CI so I did that and pushed a stubbed readme to trigger.

@phated
Copy link
Contributor Author

phated commented Apr 1, 2017

Oh, this can also replace charset option in a sense because a user can mutate the inline map to replace utf8 with some other encoding; however, that option was going to be deprecated without a replacement because the sourcemap v3 spec doesn't support any encoding except utf8.

@nmccready nmccready self-requested a review April 1, 2017 03:16
@phated
Copy link
Contributor Author

phated commented Apr 10, 2017

Thinking about this some more, I think there's more that needs to go into it because current implementation has to go after .dest() which doesn't make much sense.

nmccready
nmccready approved these changes Oct 29, 2022
View reviewed changes
Copy link

@nmccready nmccready left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

Sorry I had no clue about this PR

@phated phated marked this pull request as draft October 30, 2022 19:47
@phated phated self-assigned this Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmccready nmccready nmccready approved these changes

Assignees

@phated phated

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@phated @nmccready