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 to Node 18.15 #8631

Merged
merged 12 commits into from
Mar 10, 2023
Merged

Update to Node 18.15 #8631

merged 12 commits into from
Mar 10, 2023

Conversation

jpandersen87
Copy link
Collaborator

@jpandersen87 jpandersen87 commented Mar 7, 2023

Fixes #7078

This PR updates the required node version to 18.15.

@jpandersen87 jpandersen87 temporarily deployed to staging March 7, 2023 02:05 — with GitHub Actions Inactive
Copy link
Contributor

@snesm snesm left a 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.

@jpandersen87 jpandersen87 temporarily deployed to staging March 7, 2023 16:25 — with GitHub Actions Inactive
@jpandersen87
Copy link
Collaborator Author

Please update frontend-react/README.md as well.

Thanks for the reminder. Knew my 16.19.2 replace-all missed something!

@jpandersen87 jpandersen87 temporarily deployed to staging March 7, 2023 16:28 — with GitHub Actions Inactive
@jpandersen87 jpandersen87 temporarily deployed to staging March 7, 2023 16:29 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Mar 7, 2023

Test Results

760 tests  ±0   756 ✔️ ±0   1m 6s ⏱️ -12s
  94 suites ±0       4 💤 ±0 
  94 files   ±0       0 ±0 

Results for commit 99ed291. ± Comparison against base commit 56bac26.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 7, 2023

Integration Test Results

124 tests   124 ✔️  2m 1s ⏱️
  12 suites      0 💤
  12 files        0

Results for commit 99ed291.

♻️ This comment has been updated with latest results.

@@ -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"

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JosiahSiegel feasible?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

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 🙏

@stephenkao
Copy link

stephenkao commented Mar 7, 2023

Blech, I'm still running into the proxy issue wherein all requests to localhost:7071 are refused. BUT it works if I update proxy in package.json to http://127.0.0.1:7071 so maybe we can just update to that (or http://0.0.0.0:7071 for Docker) for a more explicit local IP address?

@etanb @penny-lischer Out of curiosity, do you run into this issue with the new version of Node?

@jpandersen87 jpandersen87 temporarily deployed to staging March 9, 2023 17:19 — with GitHub Actions Inactive
@jpandersen87 jpandersen87 changed the title Update to Node 18.14.2 Update to Node 18.15 Mar 9, 2023
@jpandersen87 jpandersen87 temporarily deployed to staging March 9, 2023 17:22 — with GitHub Actions Inactive
@jpandersen87 jpandersen87 temporarily deployed to staging March 9, 2023 17:24 — with GitHub Actions Inactive
@jpandersen87 jpandersen87 temporarily deployed to staging March 9, 2023 17:25 — with GitHub Actions Inactive
Copy link

@stephenkao stephenkao left a 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?

@jpandersen87 jpandersen87 marked this pull request as ready for review March 9, 2023 19:30
@jpandersen87 jpandersen87 requested a review from a team as a code owner March 9, 2023 19:30
Copy link
Contributor

@JosiahSiegel JosiahSiegel left a 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.

@snesm snesm temporarily deployed to staging March 10, 2023 16:05 — with GitHub Actions Inactive
@snesm snesm dismissed JosiahSiegel’s stale review March 10, 2023 16:05

Change is required for the current action to succeed

@stephenkao
Copy link

Blech, I'm still running into the proxy issue wherein all requests to localhost:7071 are refused. BUT it works if I update proxy in package.json to http://127.0.0.1:7071 so maybe we can just update to that (or http://0.0.0.0:7071 for Docker) for a more explicit local IP address?

@etanb @penny-lischer Out of curiosity, do you run into this issue with the new version of Node?

@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 127.0.0.1 or 0.0.0.0 instead of localhost? Shouldn't adversely affect anything 🤷

@jpandersen87 jpandersen87 temporarily deployed to staging March 10, 2023 17:08 — with GitHub Actions Inactive
@jpandersen87
Copy link
Collaborator Author

Blech, I'm still running into the proxy issue wherein all requests to localhost:7071 are refused. BUT it works if I update proxy in package.json to http://127.0.0.1:7071 so maybe we can just update to that (or http://0.0.0.0:7071 for Docker) for a more explicit local IP address?
@etanb @penny-lischer Out of curiosity, do you run into this issue with the new version of Node?

@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 127.0.0.1 or 0.0.0.0 instead of localhost? Shouldn't adversely affect anything 🤷

Explicit ip address commit pushed

@sonarcloud
Copy link

sonarcloud bot commented Mar 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

@stephenkao stephenkao left a 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!

@jpandersen87 jpandersen87 merged commit b9f927e into master Mar 10, 2023
@jpandersen87 jpandersen87 deleted the experience/7078/node-18 branch March 10, 2023 17:34
penny-lischer pushed a commit that referenced this pull request Mar 10, 2023
* Update to Node 18
Fixes #7078
penny-lischer added a commit that referenced this pull request Mar 10, 2023
* 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>
penny-lischer pushed a commit that referenced this pull request Mar 15, 2023
* Update to Node 18
Fixes #7078
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.

Update to Node 18
5 participants