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.

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

Make the file property optional #95

Merged
merged 1 commit into from
Feb 15, 2014
Merged

Conversation

lydell
Copy link
Contributor

@lydell lydell commented Feb 14, 2014

No description provided.

'Tried to fall back on the file property of the source map ' +
'to apply (since the second argument was omitted), but it ' +
'was not set.'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like

throw new Error(
  'SourceMapGenerator.prototype.applySourceMap requires either an explicit source file, ' +
  'or the source map\'s "file" property. Both were omitted.'
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds better. But really I'd like to deprecate omitting the second parameter. How can we warn users? console.warn? Or is that a separate pull request?

Copy link
Contributor

Choose a reason for hiding this comment

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

File an issue first to open discussion. My reaction at this moment in time is that nothing needs to be deprecated.

@fitzgen
Copy link
Contributor

fitzgen commented Feb 14, 2014

Looks good, will merge once the error message is a little more clear, as mentioned above.

@lydell
Copy link
Contributor Author

lydell commented Feb 15, 2014

I've amended your improved error message now.

fitzgen added a commit that referenced this pull request Feb 15, 2014
Make the file property optional
@fitzgen fitzgen merged commit 9315f02 into mozilla:master Feb 15, 2014
@fitzgen
Copy link
Contributor

fitzgen commented Feb 15, 2014

Thanks!

@lydell lydell deleted the optional-file branch June 2, 2014 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants