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

Node runtime and package updates #56

Merged
merged 3 commits into from
Feb 2, 2024
Merged

Conversation

jjosef
Copy link
Contributor

@jjosef jjosef commented Jan 29, 2024

Fixes #55

@rwxdash
Copy link
Member

rwxdash commented Jan 30, 2024

Hey @jjosef, thank you for opening the PR.

Can you run npm run format and commit the linting changes? Since prettier is upgraded with this PR, I assume some of the default conf have changed and causing the build to fail.

Thanks.

@jjosef
Copy link
Contributor Author

jjosef commented Jan 30, 2024

Hey @jjosef, thank you for opening the PR.

Can you run npm run format and commit the linting changes? Since prettier is upgraded with this PR, I assume some of the default conf have changed and causing the build to fail.

Thanks.

Done!

@jjosef
Copy link
Contributor Author

jjosef commented Jan 30, 2024

Also upgraded ncc and typescript packages. seems to build fine now

@rwxdash
Copy link
Member

rwxdash commented Jan 30, 2024

Thank you @jjosef. Looks good to me and the updates are probably OK, but just to be sure, have you had any chance to test the action from your fork to see things are working? If not, I'll try to make some time to test it out and approve/merge from there.

cc\ @serkan-ozal

@rwxdash
Copy link
Member

rwxdash commented Jan 30, 2024

Additionally, the release for this PR should be a major as it changes the runtime from node16 to node20.

cc\ @serkan-ozal

@jjosef
Copy link
Contributor Author

jjosef commented Jan 30, 2024

Thank you @jjosef. Looks good to me and the updates are probably OK, but just to be sure, have you had any chance to test the action from your fork to see things are working? If not, I'll try to make some time to test it out and approve/merge from there.

cc\ @serkan-ozal

Yes I am using it in production for my company currently

Copy link
Member

@rwxdash rwxdash left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you, @jjosef!

@rwxdash rwxdash changed the title #55 node and package updates Node runtime and package updates Jan 30, 2024
@fwilhe
Copy link

fwilhe commented Feb 2, 2024

Merging this PR into main would be appreciated.

We are using a fork of this action but i don't want unnecessary large diverges from upstream, so I'd like to use the commit(s) from the main branch.

Thank you

@rwxdash
Copy link
Member

rwxdash commented Feb 2, 2024

Merging it, as it looks OK at this stage and no further review required.

Thank you.

@rwxdash rwxdash merged commit a7039f9 into catchpoint:master Feb 2, 2024
1 check passed
fwilhe pushed a commit to gardenlinux/workflow-telemetry-action that referenced this pull request Feb 2, 2024
@johanneswuerbach
Copy link

@rwxdash any chance to get this released? Maybe as v2?

@serkan-ozal
Copy link
Member

Hi @johanneswuerbach,

Sorry for late reply. Yes, we should definitely release it. But I think, we can still release it as v1.
@rwxdash WDYT?

@johanneswuerbach
Copy link

johanneswuerbach commented Feb 15, 2024

Most actions I've seen released the node upgrade as major (e.g. actions/checkout or docker/build-push-action). I guess this is mainly due to the fact that actions can also be used on self-hosted runners / self-hosted github and there the new node version might not be available yet.

@rwxdash
Copy link
Member

rwxdash commented Feb 15, 2024

Hey @johanneswuerbach @serkan-ozal,

Yes, my initial thought was to release it as v2 as well since it's the convention I see from other actions. If v2 sounds good @serkan-ozal, I can prepare the action for it and we can do the release.

@serkan-ozal
Copy link
Member

Hmm, OK makes sense. @rwxdash could you please release as v2

@rwxdash
Copy link
Member

rwxdash commented Feb 15, 2024

v2 has been released. Thank you all for pushing this through!

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.

Upgrade node versions in .github actions
5 participants