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 versions in CI matrix #23354

Merged
merged 4 commits into from Oct 13, 2022
Merged

Update node versions in CI matrix #23354

merged 4 commits into from Oct 13, 2022

Conversation

xirzec
Copy link
Member

@xirzec xirzec commented Sep 29, 2022

Bumping out matrix to test up to Node 18 (latest) instead of the unstable Node 17.

Also removing Node 12 from the bottom in anticipation of our deprecating support for it after the October release.

Copy link
Member

@mikeharder mikeharder left a comment

Choose a reason for hiding this comment

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

I agree with updating v17 to v18. I would wait to remove v12 until our support officially ends.

@jeremymeng
Copy link
Contributor

Our support policy mentions 6-month after a NodeJS version reaches End of Life status. Node v12 EOL is 04/30/2022 so we are close.

@xirzec there are a couple other places where "NodeTestVersion": "12.x" is used, should those be bumped to 14.x?

@xirzec
Copy link
Member Author

xirzec commented Sep 30, 2022

@mikeharder my thinking was we're already code complete for the last version of our SDKs that will support v12, so we can hold off on merging until after they ship but really there's very little chance we're going to do anything that's 12 specific at this point.

@jeremymeng the only other spot I saw it was in some commented out config - are you seeing it still used somewhere?

@jeremymeng
Copy link
Contributor

@xirzec some packages have their own platform-matrix.json

sdk/communication/communication-phone-numbers/phone-numbers-livetest-matrix.json:24:    "NodeTestVersion": ["12.x", "14.x", "16.x", "17.x"],
sdk/communication/communication-phone-numbers/phone-numbers-livetest-matrix.json:56:      "NodeTestVersion": "12.x"
sdk/communication/communication-phone-numbers/phone-numbers-livetest-matrix.json:66:      "NodeTestVersion": "12.x",
sdk/keyvault/keyvault-admin/platform-matrix.json:13:          "NodeTestVersion": "12.x"
sdk/keyvault/keyvault-keys/platform-matrix.json:12:      "NodeTestVersion": "12.x"

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

- name: TargetDocRepoName
type: string
default: azure-docs-sdk-node
- name: Artifacts
Copy link
Member

Choose a reason for hiding this comment

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

Revert whitespace changes

Copy link
Member Author

Choose a reason for hiding this comment

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

dang prettier making this harder than it needs to be. 😄

Copy link
Member

Choose a reason for hiding this comment

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

If you are using VS Code, they have a saveWithoutFormatting command that's helpful in these cases: https://stackoverflow.com/a/71423157/3630403

Copy link
Member Author

Choose a reason for hiding this comment

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

super handy!

@@ -21,7 +21,11 @@
"SKIP_UPDATE_CAPABILITIES_LIVE_TESTS": "true"
}
},
"NodeTestVersion": ["12.x", "14.x", "16.x", "17.x"],
"NodeTestVersion": [
Copy link
Member

Choose a reason for hiding this comment

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

Revert formatting changes

@xirzec xirzec enabled auto-merge (squash) October 13, 2022 20:23
@xirzec xirzec merged commit acb2465 into Azure:main Oct 13, 2022
@xirzec xirzec deleted the updateNodeMatrix branch October 13, 2022 21:03
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.

None yet

5 participants