-
Notifications
You must be signed in to change notification settings - Fork 40
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 to Node 18.15 #8631
Update to Node 18.15 #8631
Conversation
Fixes #7078
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.
Please update frontend-react/README.md as well.
Thanks for the reminder. Knew my 16.19.2 replace-all missed something! |
@@ -28,7 +28,7 @@ runs: | |||
- name: Use Node.js ${{ matrix.node-version }} with yarn | |||
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c | |||
with: | |||
node-version: "16.19.1" | |||
node-version: "18.14.2" |
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.
I wonder if we can just use node-version-file
so we can just maintain the version in .nvmrc instead of having to keep this updated as well.
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.
@JosiahSiegel feasible?
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.
If devops approves, I feel this would be the right way to go.
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.
Also, according to the docs (see: https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#node-version-file) it can read the engines.node property in package.json so the .nvmrc file may not be needed.
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.
Oh, fantastic! Anything automatic like this would be the way to go in my opinion 🙏
Blech, I'm still running into the proxy issue wherein all requests to localhost:7071 are refused. BUT it works if I update @etanb @penny-lischer Out of curiosity, do you run into this issue with the new version of Node? |
frontend-react/src/components/MessageTracker/MessageReceivers.tsx
Outdated
Show resolved
Hide resolved
…rime-reportstream into experience/7078/node-18 merge
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.
Looking good to me! Any reason this can't be taken out of draft?
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.
I am creating a PR to update the action based on frontend-react/.nvmrc
.
Please remove the action.yml
change here, but proceed with the rest.
Change is required for the current action to succeed
@jpandersen87 I'm still running into the proxy error even with the latest minor version bump. What are your thoughts about updating that to be an explicit |
Explicit ip address commit pushed |
Kudos, SonarCloud Quality Gate passed! |
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.
Looks great to me!
* Update to Node 18 Fixes #7078
* 8395 [Manage Public Key] Add new route, and basic page * 8395 [Manage Public Key] Add new route, and basic page * 8098 generate condition filtered FHIR bundle (#8600) * Added ability to remove filtered conditions from a bundle --------- Co-authored-by: Jessica Willis <jessicawillis@navapbc.com> * Update to Node 18.15 (#8631) * Update to Node 18 Fixes #7078 * 8395 [Manage Public Key] - Changed <div> to <GridContainer>. - Removed extra <div> * 8395 [Manage Public Key] - Fix lint error --------- Co-authored-by: victor-chaparro <117938212+victor-chaparro@users.noreply.github.com> Co-authored-by: Jessica Willis <jessicawillis@navapbc.com> Co-authored-by: Joseph Andersen <josephandersen@navapbc.com> Co-authored-by: Stephen Nesman <94193373+snesm@users.noreply.github.com>
* Update to Node 18 Fixes #7078
Fixes #7078
This PR updates the required node version to 18.15.