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

Make action compatible with node 20 #10

Merged
merged 6 commits into from
Apr 24, 2024
Merged

Make action compatible with node 20 #10

merged 6 commits into from
Apr 24, 2024

Conversation

gmsantos
Copy link
Contributor

@gmsantos gmsantos commented Apr 9, 2024

Update dependencies and test actions in preparation for a new release compatible with node 20.

@gmsantos gmsantos self-assigned this Apr 9, 2024
Copy link

@klemmster klemmster left a comment

Choose a reason for hiding this comment

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

https://github.com/hellofresh/action-compile-job-status/blob/master/action.yml#L20
needs to be changed to:

runs:
  using: 'node20'
  main: dist/index.js

Docs

@klemmster
Copy link

Copy link

@klemmster klemmster left a comment

Choose a reason for hiding this comment

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

How did you update the dist?
It's a bit strange that the overall lines added is quite big. Looks like tree-shaking might be missing?

Could we add the compilation to the workflows like that.
so it automatically updates in PRs and is documented that way, too.

@gmsantos
Copy link
Contributor Author

How did you update the dist?

From local, I ran the steps defined at the workflow in repository:

npm ci
npm run package

I created #11 to track this.

It's a bit strange that the overall lines added is quite big. Looks like tree-shaking might be missing?

We use @vercel/ncc to create the dist script. Unfortunately, it does not support tree-shaking. We could try a different bundler (like rollup) that supports tree-shaking to make the dist package smaller (tracked in #12).

Copy link

@klemmster klemmster 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, nothing more to complain about.

Lets give it a try

@gmsantos gmsantos merged commit dcf4e5d into master Apr 24, 2024
9 checks passed
@gmsantos gmsantos deleted the node-20-build branch April 24, 2024 10:18
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