Skip to content

Change node version for action to node20 - #866

Merged
dmitry-shibanov merged 8 commits into
actions:mainfrom
dmitry-shibanov:v-dmshib/action-to-node20
Oct 23, 2023
Merged

Change node version for action to node20#866
dmitry-shibanov merged 8 commits into
actions:mainfrom
dmitry-shibanov:v-dmshib/action-to-node20

Conversation

@dmitry-shibanov

@dmitry-shibanov dmitry-shibanov commented Oct 9, 2023

Copy link
Copy Markdown
Contributor

Description:
In scope of this pull request we switch node version of the action to node20.
Related issue:
#850

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@dmitry-shibanov
dmitry-shibanov requested a review from a team as a code owner October 9, 2023 13:59
);
} else {
core.info(err.message);
core.info((err as Error).message);

ghost Oct 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the compiler does not detect the 'Error' class, shouldn't we dynamically check it, either with a type guard or as simple as (err.message && message || something)?

ghost Oct 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it is unnecessary because if it was not an error initially, the code related to core.info will fail because it accepts only strings. It is unexpected behaviour that string is thrown instead of Error.

@dmitry-shibanov
dmitry-shibanov merged commit 54534a2 into actions:main Oct 23, 2023
@MikeMcC399 MikeMcC399 mentioned this pull request Oct 23, 2023
@MikeMcC399

ghost commented Oct 23, 2023

Copy link
Copy Markdown
Contributor

@dmitry-shibanov

  • I don't see any update to the README to list actions/setup-node@v4
  • At the moment, there is only a tag v4.0.0.
    There is however not yet a tag v4 (nor a branch v4). Edit: done.

Are these points separately planned?

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.

8 participants