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 Node.js version on CI #432

Closed
wants to merge 3 commits into from

Conversation

sosukesuzuki
Copy link
Contributor

context: #265 (comment)

  • Make it refer .nvmrc from GitHub Actions workflows
  • Update Node.js version from 14.15 to 18.17 because 18.x is current LTS version

Signed-off-by: Sosuke Suzuki <sosuke.suzuki@dr-ubie.com>
Signed-off-by: Sosuke Suzuki <sosuke.suzuki@dr-ubie.com>
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Oct 26, 2023
@agilgur5
Copy link
Contributor

context: #265 (comment)

Hey sorry, I totally didn't see this PR; thought you would make the change in that PR. This works too, just that #265 will need rebasing after this

  • Update Node.js version from 14.15 to 18.17 because 18.x is current LTS version

This is good to have, just will have to make sure nothing else breaks from the more significant upgrade

Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM if CI build passes. Thanks for updating this!

@agilgur5 agilgur5 requested a review from rbreeze October 29, 2023 03:50
@agilgur5 agilgur5 added type/dependencies Pull requests that update a dependency file and removed problem/stale This has not had a response in some time labels Oct 29, 2023
@terrytangyuan terrytangyuan enabled auto-merge (squash) November 15, 2023 20:51
@agilgur5
Copy link
Contributor

agilgur5 commented Jan 4, 2024

It looks like the build step never ran for some reason 😕

@terrytangyuan are you able to merge master into this PR so that the checks (hopefully) run?

@terrytangyuan
Copy link
Member

triggered

@agilgur5 agilgur5 self-assigned this Jan 5, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented Jan 6, 2024

Thanks Terry! The build ran this time, but failed due to the infamous digital envelope routines::unsupported OpenSSL version error. I'll work on that offline and submit a superseding PR

@agilgur5 agilgur5 added the javascript Pull requests that update Javascript code label Jan 6, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented Jan 6, 2024

So we could use the OpenSSL legacy provider to workaround the issue, as we did in argoproj/argo-workflows#11410 and argoproj/argo-workflows#11461, but I'd prefer to just update the deps so we don't have the OpenSSL vuln anyway (although it only impacts local dev, so not a huge deal).

Unfortunately, the main dep that needs upgrading for that is Webpack, which is only used by Storybook in this repo. The problem there is that we're on an old version of Storybook, v6, which hasn't been updated since 2022. Upgrading to Storybook v7 is fairly non-trivial, not the least of which because there are actually three Storybook builds in this repo: for v1, v2, and antd (also there's a separate yarn.lock for v2 for some reason, which I imagine might've been an accident, but idk -- EDIT: See #510 regarding the second yarn.lock, it is extraneous/no longer used).
The main two active builds have two different configurations as well, and heck even configuration files themselves, with v1's being 6+ years old and still using Storybook v5 syntax etc.

I tried updating the deps and everything to at least get v2 working, but kept getting build error after build error fixing one-by-one. So this may very well not be worth upgrading given the current state of the repo per #453.

Sooo I might just enable the OpenSSL legacy provider workaround instead and at least get Node updated to non-EoL (Storybook et al will have to come later I guess 😕 (if ever))

@agilgur5
Copy link
Contributor

node-version-file was specified in #509, and Node upgrade to v20 is completed in #536

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Apr 15, 2024
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Apr 15, 2024
@agilgur5 agilgur5 added the solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) label May 12, 2024
@agilgur5 agilgur5 closed this in #536 Jun 3, 2024
auto-merge was automatically disabled June 3, 2024 16:55

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) type/dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants