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

Use actions/toolkit #30

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

LecrisUT
Copy link

@LecrisUT LecrisUT commented Jun 8, 2023

The github actions documentation encourages using actions/toolkit

Ok, this one became more complicated than initially thought. Changing from a composite action to a js action requires a compilation step of a dist/index.js file. Not sure if it's worth having the additional steps.


📚 Documentation preview 📚: https://readthedocs-preview--30.org.readthedocs.build/en/30/

Ensures the PR version of the repo is used

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@humitos
Copy link
Member

humitos commented Jun 8, 2023

Hi! I'm a little new on the GitHub Actions world. Can you please explain to me what are the benefits of actions/toolkit over the current pattern we are following here?

@LecrisUT
Copy link
Author

LecrisUT commented Jun 8, 2023

Initially I thought it can make it cleaner and more in line with upstream documentation, but having to enforce a compilation workflow is a bit of a disadvantage. Maybe if we push compilation commits automatically, it wouldn't be that bad.

Other than that there can be some benefits:

  • One less checkout action to call and manage. All source is directly accessed by the current action caller. This also ensures that other gh action callers can use a fork version for testing
  • Simpler reusability
  • Allows to use external libraries from npm
  • Unit testing eventually

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