Skip to content

RUM-6120 Upgrade Github action to run on Node.js 20#173

Merged
maxep merged 5 commits intomainfrom
upgrade-node
Sep 16, 2024
Merged

RUM-6120 Upgrade Github action to run on Node.js 20#173
maxep merged 5 commits intomainfrom
upgrade-node

Conversation

@mariedm
Copy link
Member

@mariedm mariedm commented Sep 12, 2024

This PR updates the repository to use Node.js 20 in alignement with GitHub’s recent transition to Node.js 16 for GitHub Actions.

Given that we were already using Node.js 18 in the dependencies, and that Github Action doesn't support Node.js 18, it made sense to move directly to Node.js 20.

@mariedm mariedm changed the title Upgrade Github action to run on Node.js 16 RUM-6120 Upgrade Github action to run on Node.js 16 Sep 12, 2024
@mariedm mariedm changed the title RUM-6120 Upgrade Github action to run on Node.js 16 RUM-6120 Upgrade Github action to run on Node.js 18 Sep 12, 2024
// Given
jest.spyOn(core, 'getInput').mockImplementation(() => 'foo')
const setFailedMock = jest.spyOn(core, 'setFailed')
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation(() => {})

Choose a reason for hiding this comment

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

🔴 Code Quality Violation

Suggested change
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation(() => {})
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation(() => {/* empty */})
Empty statement. (...read more)

Empty or non-functional blocks in the code can be misleading and lead to maintenance difficulties. They can also lead to a false sense of security or functionality. While they may not directly introduce security issues, their presence can suggest that some logic or error handling is implemented when it is not.

View in Datadog  Leave us feedback  Documentation

Copy link
Member

@maxep maxep Sep 12, 2024

Choose a reason for hiding this comment

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

/nit This suggestion is better than a console call IMHO

// Given
const setFailedMock = jest.spyOn(core, 'setFailed')
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation((message) => {
console.log(`Mocked setFailed called with message: ${message}`);

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unexpected console statement. (...read more)

Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.

View in Datadog  Leave us feedback  Documentation

jest.spyOn(core, 'getInput').mockImplementation(() => 'foo')
const setFailedMock = jest.spyOn(core, 'setFailed')
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation((message) => {
console.log(`Mocked setFailed called with message: ${message}`);

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unexpected console statement. (...read more)

Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.

View in Datadog  Leave us feedback  Documentation

const setFailedMock = jest.spyOn(core, 'setFailed')
/* eslint-disable no-console */
const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation((message) => {
console.log(`Mocked setFailed called with message: ${message}`)

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unexpected console statement. (...read more)

Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.

View in Datadog  Leave us feedback  Documentation

@mariedm mariedm force-pushed the upgrade-node branch 2 times, most recently from 949aeeb to c46e789 Compare September 13, 2024 12:24
@mariedm mariedm changed the title RUM-6120 Upgrade Github action to run on Node.js 18 RUM-6120 Upgrade Github action to run on Node.js 20 Sep 13, 2024
@mariedm mariedm marked this pull request as ready for review September 13, 2024 14:02
@mariedm mariedm requested a review from a team as a code owner September 13, 2024 14:02
@mariedm mariedm requested a review from maxep September 16, 2024 08:18
@maxep maxep merged commit c0c21a2 into main Sep 16, 2024
@maxep maxep deleted the upgrade-node branch September 16, 2024 14:08
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