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

Bump node.js version to 18.17.0 [AP-1081] #1392

Merged
merged 8 commits into from
Dec 18, 2023
Merged

Conversation

woodfell
Copy link
Contributor

@woodfell woodfell commented Dec 14, 2023

Description

@swift-nav/devinfra

Bump node.js version in docker image to 18.17.0

This resolves the docker image build failures seen on #1373 and allows a freshly built docker image to build the javascript bindings

API compatibility

Does this change introduce a API compatibility risk?

No

API compatibility plan

If the above is "Yes", please detail the compatibility (or migration) plan:

N/A

JIRA Reference

https://swift-nav.atlassian.net/browse/AP-1081

@woodfell woodfell marked this pull request as ready for review December 14, 2023 03:07
@woodfell woodfell requested a review from a team as a code owner December 14, 2023 03:07
@woodfell woodfell changed the title Bump node.js version to 20.10.0 LTS [AP-1081] Bump node.js version to 18.17.0 [AP-1081] Dec 14, 2023
@woodfell woodfell requested a review from a team as a code owner December 14, 2023 10:09
Copy link

sonarcloud bot commented Dec 14, 2023

Quality Gate Passed Quality Gate passed for 'libsbp-c'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

the patch numbers got all goofed. I will try and fix them from you but i don't want to merge it as is.

@woodfell
Copy link
Contributor Author

the patch numbers got all goofed. I will try and fix them from you but i don't want to merge it as is.

Please do, I spent most of last night trying to do this and gave up. The generator seems to come up with different values when run in CI versus locally. The state of this PR is just what gets through the check

@pcrumley
Copy link
Contributor

the patch numbers got all goofed. I will try and fix them from you but i don't want to merge it as is.

Please do, I spent most of last night trying to do this and gave up. The generator seems to come up with different values when run in CI versus locally. The state of this PR is just what gets through the check

the issue is probably that the 5.0.4 tag is on an detached branch... maybe it's better to just have a commit that we take before the release which gets the version numbers aligned.

I also recommend you add a #no_auto_pr tag to the PR description so it won't bump a bunch of submodules in our codebase

Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

Confirming plan here, since v5.0.4 is on a non master branch we will take this commit back to 5.0.3 and then jump ahead to 5.0.5 next release so no tags are clobbered

@woodfell woodfell merged commit 1bfe255 into master Dec 18, 2023
32 of 33 checks passed
@woodfell woodfell deleted the woodfell/bump_node_version branch December 18, 2023 01:31
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.

2 participants