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

Update javascript libraries in various places #3685

Merged
merged 5 commits into from
Sep 20, 2019
Merged

Update javascript libraries in various places #3685

merged 5 commits into from
Sep 20, 2019

Conversation

dholbach
Copy link
Contributor

Update javascript libraries in various places, needs newer node.

[daniel@reef scope ]$ cat client/Dockerfile 
# Changes to this file will not take effect in CI
# until the image version in the CI config is updated. See
# https://github.com/weaveworks/scope/blob/master/.circleci/config.yml#L11
FROM node:8.12
[...]

Copy link
Contributor

@qiell qiell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dholbach
Copy link
Contributor Author

@fbarl Can you advise on the update for the CI related bits?

@fbarl
Copy link
Contributor

fbarl commented Sep 18, 2019

Can you advise on the update for the CI related bits?

So, the UI image that CircleCI currently uses is weaveworks/scope-ui-build:master-c0b60a16 which seems to be about a year old. The last node version bump happened in #3231 also about a year ago, so that seems right.

I'm not sure this is the best way to proceed but what we could do for sure is:

  1. Merge a separate node version bump PR into master first (would be a forced merge as the CircleCI would fail with the same errors as here)
  2. I can build that master locally and push the new weaveworks/scope-ui-build image to Dockerhub manually
  3. We update .circleci/config.yml#L11 here with the new Dockerhub image and fix the CircleCI errors

This doesn't sound like the optimal approach though and I wonder if @bboreham knows of any better way to proceed. If not, we could follow those 3 steps above and get it out of the way.

@bboreham
Copy link
Collaborator

The change to use specific tags for CircleCI build image was in #3333, but I can't see that I explained it very well.
The build image has always been hand-pushed from someone's machine.
I suggest building and pushing from this branch, then add a commit to this PR to make config.yml use that image, then after it is merged we can update with a tag from master.

@fbarl fbarl self-assigned this Sep 18, 2019
@fbarl
Copy link
Contributor

fbarl commented Sep 19, 2019

Checklist (suggested by @bboreham):

  • Build and push the image from this branch to Dockerhub
  • Add a commit that updates config.yml with the new image tag + comment about reverting
  • Open a new PR which switches back to the new master tag after this PR is merged

@dholbach dholbach requested a review from fbarl as a code owner September 19, 2019 12:35
@fbarl fbarl removed their request for review September 19, 2019 12:36
@fbarl
Copy link
Contributor

fbarl commented Sep 19, 2019

Going to merge this PR after #3689 is merged.

@fbarl fbarl merged commit d1bb021 into master Sep 20, 2019
@fbarl fbarl deleted the js-updates branch September 20, 2019 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants