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.

Sign up for GitHub

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

Jump to bottom

Bump node version to 16.13.0 #5934

Merged
merged 7 commits into from
Mar 31, 2023
Merged

Bump node version to 16.13.0 #5934

merged 7 commits into from
Mar 31, 2023

Conversation

gantunesr
Copy link
Member

No description provided.

sethkfman
sethkfman reviewed Mar 9, 2023
View reviewed changes
package.json Show resolved Hide resolved
@gantunesr gantunesr requested a review from sethkfman March 10, 2023 03:42
@gantunesr gantunesr marked this pull request as ready for review March 10, 2023 03:42
@gantunesr gantunesr requested a review from a team as a code owner March 10, 2023 03:42
@gantunesr gantunesr added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 10, 2023
@Gudahtt
Copy link
Member

Gudahtt commented Mar 16, 2023

Just curious: was there a reason for choosing v16.13 specifically, rather than v16?

@Gudahtt
Copy link
Member

Gudahtt commented Mar 16, 2023

Could you update the README as well? It references version 14 in the setup instructions

@gantunesr
Copy link
Member Author

Hey @Gudahtt, I just now realized you left some comments,

Just curious: was there a reason for choosing v16.13 specifically, rather than v16?

I think v16 should work. The reason is that we need v16.13 for the snaps system integration into mobile

Could you update the README as well? It references version 14 in the setup instructions

Yes

@gantunesr
Copy link
Member Author

Done @Gudahtt

Gudahtt
Gudahtt approved these changes Mar 30, 2023
View reviewed changes
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

leotm
leotm approved these changes Mar 31, 2023
View reviewed changes
Copy link
Member

@leotm leotm left a comment

Choose a reason for hiding this comment

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

nice ^ we officially make the switch to .node-version and set v16 in 0.70
but our 0.66.5 .nvmrc ahead on 16's perfectly fine

but after SES/lockdown likely needs work LavaMoat-side to sandbox compile/run time
since we've been on v14 only couple months - i'll check w the team to plan/start early

i also bumped to v18 fb picked for 0.71+ (but got few months till Node 16 EOL: 2023-09-11)
our 0.71.4 branch (i'm using for lockdown) is behind on v16.14.0 so also checking this is ok

Node 19's been around few months too (got unofficially working for RN)
so hopefully can bundle the LavaMoat work to support 16/18/19 at same time

@gantunesr gantunesr added No QA Needed Apply this label when your PR does not need any QA effort. release-6.4.0 Issue or pull request that will be included in release 6.4.0 and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 31, 2023
@gantunesr gantunesr merged commit 8aac1e0 into main Mar 31, 2023
@gantunesr gantunesr deleted the bump-node branch March 31, 2023 16:37
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leotm leotm leotm approved these changes

@Gudahtt Gudahtt Gudahtt approved these changes

@sethkfman sethkfman Awaiting requested review from sethkfman

Assignees
No one assigned
Labels
No QA Needed Apply this label when your PR does not need any QA effort. release-6.4.0 Issue or pull request that will be included in release 6.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gantunesr @Gudahtt @leotm @sethkfman