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

Updated all dependencies. #4125

Closed
wants to merge 2 commits into from
Closed

Conversation

bhamodi
Copy link
Contributor

@bhamodi bhamodi commented Jun 14, 2015

Did this via npm using the following commands: npm update --save and npm update --save-dev.

This will ensure we have the latest versions of every dependency at this moment. If all tests pass, I would think it is safe to believe that there are no regressions.

@magsout
Copy link

magsout commented Jun 14, 2015

It is for this reason that there is ^ before each dependency. See https://docs.npmjs.com/misc/semver#caret-ranges-1-2-3-0-2-5-0-0-4

@chicoxyzzy
Copy link
Contributor

actually you should do npm outdated

@bhamodi
Copy link
Contributor Author

bhamodi commented Jun 14, 2015

Good catch @chicoxyzzy - I've actually updated all the dependencies in the latest commit.

@zpao
Copy link
Member

zpao commented Jun 15, 2015

You can't do this blindly. There's actually a reason we're stuck at some of our dependencies (eg, by updating sauce-tunnel you actually broke some of our testing, npm-jsx_orphaned_brackets_transformer actually needs the specific version of jstransform that it specified).

For many of the other things, the ^ range will install the latest version and I don't feel the need to bump everything. Here's what npm outdated --depth=0 tells me after a fresh install:

Package                Current  Wanted      Latest  Location
coverify                 1.0.7   1.0.7       1.3.3  coverify
browserify               9.0.8   9.0.8      10.2.4  browserify
eslint                  0.22.1  0.22.1      0.23.0  eslint
eslint-tester            0.7.0   0.7.0       0.8.0  eslint-tester
grunt-contrib-connect    0.6.0   0.6.0      0.10.1  grunt-contrib-connect
sauce-tunnel             1.1.2   1.1.2       2.2.3  sauce-tunnel
typescript               1.4.1   1.4.1  1.5.0-beta  typescript
wd                      0.2.27  0.2.27      0.3.12  wd

If you really want to go update everything, that's a great goal. But I won't accept it blind like this. Doing it in pieces is ok. Eg, update the server-side rendering example and test it. We need to know you tested it and it worked before we ship it to a bunch of people. I'm going to close this PR out as is as a result. But if you do want to do this, I'd expect to see at least 4 separate PRs (or 1 big PR with multiple commits), each with an complete test plan.

@zpao zpao closed this Jun 15, 2015
@bhamodi
Copy link
Contributor Author

bhamodi commented Jun 15, 2015

Thanks for the detailed response @zpao. You're right.
I'll look into possibly doing this in smaller, more test-able pieces. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants