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 to node16 and cleanup node_modules #9

Merged
merged 8 commits into from
Nov 1, 2022
Merged

Update to node16 and cleanup node_modules #9

merged 8 commits into from
Nov 1, 2022

Conversation

nirinchev
Copy link
Member

This updates actions to node16 as node12 is deprecated. Additionally, I noticed we're committing node_modules which is a bad practice. I've deleted the folder and added a .gitignore.

@bwachter
Copy link
Collaborator

https://docs.github.com/en/actions/creating-actions/creating-a-javascript-action#commit-tag-and-push-your-action-to-github

"GitHub downloads each action run in a workflow during runtime and executes it as a complete package of code before you can use workflow commands like run to interact with the runner machine. This means you must include any package dependencies required to run the JavaScript code."

@nirinchev
Copy link
Member Author

Yes, the recommended way to do it is via ncc, not by checking in node_modules. I've added a build script that compiles the dependencies into a single file.

@nirinchev nirinchev temporarily deployed to production October 31, 2022 09:42 Inactive
@nirinchev nirinchev requested a review from bwachter October 31, 2022 11:26
@bwachter
Copy link
Collaborator

Looks good to me. Switching bool input handling over to getBooleanInput is tracked in #11 - my best guess still is that something weird was going on in input validation there.

@nirinchev nirinchev temporarily deployed to production October 31, 2022 14:22 Inactive
@nirinchev nirinchev merged commit c929c9a into master Nov 1, 2022
@nirinchev nirinchev deleted the ni/node16 branch November 1, 2022 19:57
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