Skip to content

Expose all actions toolkit packages #120

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

Closed
wants to merge 1 commit into from
Closed

Expose all actions toolkit packages #120

wants to merge 1 commit into from

Conversation

MatisseHack
Copy link
Contributor

Some toolkit packages are already individually passed to the script as arguments (@actions/core and @actions/io), but there are additional toolkit packages that could be helpful when writing scripts.

This PR adds all toolkit modules to an actions module that is then passed to the script. To use the @actions/exec module, one could simply write:

await actions.exec.exec('node index.js');

@MatisseHack MatisseHack requested a review from a team February 24, 2021 20:52
@MatisseHack
Copy link
Contributor Author

Looks like I would need to add some more licenses to satisfy licensed. I don't want to clutter up this PR unnecessarily so I can add the missing licenses once/if you're open to merging this in.

@Legion2
Copy link

Legion2 commented Feb 26, 2021

Is there a workaround to use the other packages without this PR?

@MatisseHack
Copy link
Contributor Author

MatisseHack commented Feb 26, 2021

You might be able to npm install the packages in a previous step and then require them in this step, but I haven't tested it.

@joshmgross
Copy link
Contributor

👋 Thanks for the submission @MatisseHack. I'm a little concerned about the significant increase in this action's size from including all of these packages.

Is there a specific use case for each of these? For packages like @actions/cache and @actions/artifact, I would expect a fair bit of code to use those fully and would be better off used in their own action.

@MatisseHack
Copy link
Contributor Author

True, it does increase the size by several factors. Would minifying be enough to assuage your concerns? That gets the 2.5 MB file down to 1.1 MB. Still larger than the original 210 KB file, but maybe within reason?

I actually only want access to the @actions/glob package at the moment, but I can imagine wanting access to some others in the future. I agree that @actions/cache and @actions/artifact might be less useful for short scripts, but I included them for completeness. I think this action is a fantastic way to explore the toolkit and prototype a new action so having access to both the full octokit and the full actions toolkit just seemed right 🙂

That being said, I don't have any personal need for those two packages at the moment so I'm open to removing them if needed.

@joshmgross
Copy link
Contributor

@MatisseHack Could we only add @actions/glob and keep the naming consistent with the other packages (rather than using the actions namespace)?

@MatisseHack
Copy link
Contributor Author

Forgot to mention this in the PR description, but part of the motivation behind using a namespace was to eventually simplify the script parameters to just: require, octokit, and actions. These simplified parameters would also avoid the confusion of github currently referring to an octokit/core.js instance rather than @actions/github.

But obviously that would be a breaking change, which also makes a bit less sense if the actions namespace doesn't contain all the toolkit packages.

I went ahead and created #127 with just the globbing package added.

@joshmgross
Copy link
Contributor

These simplified parameters would also avoid the confusion of github currently referring to an octokit/core.js instance rather than @actions/github

Yeah I can definitely see how that would be confusing 😅

Going to close this PR out, I've merged #127 and release a new version soon

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.

3 participants