Skip to content

grunt npmcopy is not a noop on master #2160

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

Closed
mgol opened this issue Mar 24, 2015 · 7 comments
Closed

grunt npmcopy is not a noop on master #2160

mgol opened this issue Mar 24, 2015 · 7 comments
Labels

Comments

@mgol
Copy link
Member

mgol commented Mar 24, 2015

grunt npmcopy on master modifies Sinon fake timers heavily. Perhaps the npmcopy task wasn't run after one of recent Sinon updates?

@mgol mgol added the Build label Mar 24, 2015
@mgol mgol added this to the 3.0.0 milestone Mar 24, 2015
@scottgonzalez
Copy link
Member

See 9101704.

@mgol
Copy link
Member Author

mgol commented Mar 24, 2015

@scottgonzalez Thanks. Maybe having grunt npmcopy as a postinstall script wouldn't be bad. I've heard postinstall scripts are discouraged although I don't know why.

@scottgonzalez
Copy link
Member

That would potentially put you in a dirty state immediately though, right? That doesn't sound great.

@mgol
Copy link
Member Author

mgol commented Mar 24, 2015

If you touch package.json, it's good that it gets you in a dirty state so that you don't forget to commit everything. If files match, this will be a noop.

@scottgonzalez
Copy link
Member

Assuming the person making the change is manually changing package.json and then using npm install. If the changes are made with npm install --save-dev some-package, then the postinstall script won't run.

@timmywil
Copy link
Member

A postinstall would help us remember, but we don't want users and one-time contributors to have to deal with it.

@mgol
Copy link
Member Author

mgol commented Mar 24, 2015

If the changes are made with npm install --save-dev some-package, then the postinstall script won't run.

Right. :/ Not a good idea then.

timmywil added a commit that referenced this issue Nov 10, 2015
@dmethvin dmethvin removed this from the 3.0.0 milestone Jan 7, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

4 participants