-
Notifications
You must be signed in to change notification settings - Fork 20
Split JS tests into a separate build #1070
base: develop
Are you sure you want to change the base?
Conversation
I'm not sure that putting a bunch of things in |
"vumigo_v01": "~0.1.22", | ||
"vumigo_v02": "~0.2" | ||
}, | ||
"optionalDependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Failure to install optional dependencies doesn't fail the install process, which might be annoying -- https://docs.npmjs.com/files/package.json#optionaldependencies.
Just mentioning this in case others feel it's a show stopper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I don't think we can consider these deps optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only alternatives I can think of are listing them in a file and running cat extra-node-deps.txt | xargs npm install
or something, or having a second package.json
with the extra deps it in that we can install separately. I really don't want to leave these in dependencies
, because installing libxmljs
takes rather a long time.
(FWIW, these deps are optional in the sense that nothing in vumi-go depends on them -- they're only required by some jsbox apps.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, ideally those deps shouldn't be in the vumi-go repo, they're needed in production, not here. A better solution would be to have a separate package.json
for production with these dependencies in that doesn't live in this repo. Maybe we should do something like that instead? In the mean time, I do think its going to be safer to not make these deps optional here. There's potential for deploys to go bad if dependencies that need to be deployed aren't deployed.
They often fail spuriously (yay npm) and don't depend on Python version. Putting them in their own build should give us a speed boost and make it easier to rerun just the broken things.