Release: set version in source for releases #2981
Conversation
|
By analyzing the blame information on this pull request, we identified @markelog, @scottgonzalez and @jeresig to be potential reviewers |
|
Could you also replace EDIT: although, since it's not used in AMD mode, maybe we should just not publish that file to npm/bower? |
| @@ -1,3 +1,4 @@ | |||
| var fs = require( "fs" ); | |||
There was a problem hiding this comment.
Looks inconsistent, other requires in the module.exports scope
There was a problem hiding this comment.
I make a distinction between modules that should be required when the exported function is called and ones that don't need to be.
There was a problem hiding this comment.
Feels very specific as "gotcha" style rule i.e. hard to follow, don't recall we do this in anywhere else.
|
@mgol I think it's okay to publish files that aren't used. If anyone tries to use wrapper.js, they'll find out quick it doesn't work, but I doubt anyone will actually try that. I don't mind leaving it out, but not copying it would mean switching to a list of all of the files in the src folder, rather than just copying the src folder (at least for bower), and I'd rather not do that. |
You could use glob patterns & negate this one file out without major changes. But I don't insist. ;) So, are you for leaving it inside the published package with the non-replaced |
For now, I'd be open to a later patch excluding the file, but it's not high priority. |
Fixes gh-2979