Skip to content
This repository has been archived by the owner on Mar 17, 2023. It is now read-only.

Downstream changes: Pin dependency versions #18

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

VaishnaviHire
Copy link

Cherry picked commits:

  • Update configurable-http-proxy to v4.2.3 for CPaaS
  • Update version of follow-redirects
  • Update color-string dependency to 1.5.5
  • Update color-name dependency version
  • Update fecha dependency to 4.2.1

@tmckayus
Copy link

We can close this PR as well.

The issue is that in .s2i/bin/assemble, we have this code

# Install npm packages required by JupyterHub.

echo " -----> Installing npm packages."

npm install -g configurable-http-proxy

That npm install is not legal downstream, the dependencies need to be known ahead of time. But for upstream builds, this is fine so the package.json and package-lock.json are not needed.

@tmckayus
Copy link

It might not hurt to actually have them, but that is a decision for an npm expert :)

@VaishnaviHire
Copy link
Author

#18 (comment)

@vpavlin @mroman-redhat Any thoughts ?

@tmckayus
Copy link

@harshad16 what do you think?

@harshad16
Copy link

so this was just need for the cpaas build.
It is not needed in the upstream.
i m debating if this needed for consistency , but then we would have to update the assemble script
https://github.com/opendatahub-io/jupyterhub-quickstart/blob/d7b91a5d3936e5f8651179f5eaef416e5b9f82e3/.s2i/bin/assemble#L19
to npm install -g
so that it uses package.json , or else these would be just present in the repo and not be used.

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

Successfully merging this pull request may close these issues.

3 participants