mozilla / activity-stream

A refreshed "new tab page" for Firefox
Mozilla Public License 2.0
183 stars 113 forks source link

Bug 1597301 - Greening up dev deps #5384

Open gvn opened 5 years ago

gvn commented 5 years ago

next-update --test './node_modules/.bin/npm-run-all test buildmc' --type dev

1 failed upgrade: karma-sinon 1.0.5 -> 1.8.0

Ran on OSX via:

node @ v8.16.2
npm @ 6.9.2
gvn commented 5 years ago

Looks like Prettier update blew up, perhaps due to new rules despite being a minor bump? Should I separate that out into its own PR with files prettified to pass?

piatra commented 5 years ago

Looks like Prettier update blew up, perhaps due to new rules despite being a minor bump? Should I separate that out into its own PR with files prettified to pass?

Yes I think it would be best to split it out if it involves code changes. On second thought we might want to confirm/compare with the m-c version. Are we at risk of exporting something not compatible m-c? In that case we should hold off on the upgrade.

Curious why karma-sinon failed, is that easily fixable as well?

@gvn our package.json specifies npm 6.9. I don't think it's a big difference in the end result but it would mean that everyone else doing npm install would have a different package-lock.json.

gvn commented 5 years ago

@gvn our package.json specifies npm 6.9. I don't think it's a big difference in the end result but it would mean that everyone else doing npm install would have a different package-lock.json.

I accidentally put the wrong nvm profile in when I was writing that comment. Correct versions are now listed! (npm was/is at 6.9.2)

piatra commented 5 years ago

Hm strage https://www.npmjs.com/package/karma-sinon There is no 1.8.0 release.

dmose commented 4 years ago

Sorry for the long delay getting to this!

@gvn, I had not seen next-update before, that's very cool!

Prettier makes changes with updated versions (see https://prettier.io/docs/en/install.html). This means that we need to be using the same version as mozilla-central, so it can't be upgraded. We should probably add a package.json comment to that effect to avoid future confusion:

    "//": "Prettier must be the same version as the one in mozilla-central's top-level package.json",
    "prettier": "1.17.0"

I suppose we could add (meta-data) [https://github.com/bahmutov/next-update#ignoring-or-skipping-some-modules] to make next-update do the right thing also.