Skip to content

/ jquery

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

Jump to bottom

grunt npmcopy is not a noop on master #2160

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
Build

Comments

@mgol
Copy link
Member

@mgol 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

@scottgonzalez scottgonzalez commented Mar 24, 2015

See 9101704.

mgol referenced this issue Mar 24, 2015
@timmywil
Build: update node dependencies
Loading status checks…
9101704
@mgol
Copy link
Member Author

@mgol 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

@scottgonzalez scottgonzalez commented Mar 24, 2015

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

@mgol
Copy link
Member Author

@mgol 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

@scottgonzalez scottgonzalez commented Mar 24, 2015

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

@timmywil timmywil commented Mar 24, 2015

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 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 timmywil closed this in 98c25b7 Mar 24, 2015
timmywil added a commit that referenced this issue Nov 10, 2015
@timmywil @markelog
Build: save sinon update for later
d810ab1 
Fixes gh-2160
@dmethvin dmethvin removed this from the 3.0.0 milestone Jan 7, 2016
@cssmagic cssmagic mentioned this issue May 18, 2016
Open
@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.
Assignees
No one assigned
Labels
Build
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@scottgonzalez @dmethvin @timmywil @mgol