-
Notifications
You must be signed in to change notification settings - Fork 88
Add action typing #250
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
base: main
Are you sure you want to change the base?
Add action typing #250
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for action typing by integrating typing metadata and corresponding updates to the GitHub Action configuration. Key changes include:
- Updating the JavaScript script to inject generated permission inputs into action.yml and action-types.yml.
- Adding an action-types.yml file with generated typing definitions.
- Updating the README and adding a GitHub workflow to validate action typing.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
scripts/update-permission-inputs.js | Renamed variables for consistency and added logic to update YAML files |
action-types.yml | Added generated permission types to support action typing |
README.md | Updated documentation to mention type-safe workflows support |
.github/workflows/check-action-typing.yml | Added workflow to validate the action typing definitions |
Comments suppressed due to low confidence (1)
scripts/update-permission-inputs.js:36
- [nitpick] Consider adding error handling or a verification step to ensure that the YAML markers ( and ) are found, to avoid silent failures if the markers are missing.
const updatedActionYamlContent = actionYamlContent.replace(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few nits for now. I didn't have a chance to review @typesafegithub in detail yet. Love the effort though!
5269c8a
to
3e48bb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds first‐class support for action typings, enabling a type-safe way to define GitHub Action inputs/outputs using a separate YAML file and a corresponding GitHub workflow to validate it.
- Renames variables and updates the replace logic in the JavaScript helper for permissions inputs.
- Adds a new action-types.yml with generated permission type definitions.
- Updates the README and introduces a new workflow to check the action typing.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
scripts/update-permission-inputs.js | Renamed variables and updated the regex-based replace logic. |
action-types.yml | Added auto-generated permission types based on user schema. |
README.md | Included documentation on using action typings with the DSL. |
.github/workflows/check-action-typing.yml | Added a GitHub workflow for validating the action typings. |
|
||
// In the action.yml file, replace the content between the `<START GENERATED PERMISSIONS INPUTS>` and `<END GENERATED PERMISSIONS INPUTS>` comments with the new content | ||
const updatedActionsYamlContent = actionsYamlContent.replace( | ||
const updatedActionYamlContent = actionYamlContent.replace( | ||
/(?<=# <START GENERATED PERMISSIONS INPUTS>)(.|\n)*(?=# <END GENERATED PERMISSIONS INPUTS>)/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the complex regex pattern used in the replace method into a well-named constant or helper function to improve readability and maintainability.
/(?<=# <START GENERATED PERMISSIONS INPUTS>)(.|\n)*(?=# <END GENERATED PERMISSIONS INPUTS>)/, | |
PERMISSIONS_INPUTS_REGEX, |
Copilot uses AI. Check for mistakes.
In general, once we start reviewing, please don't force push, it makes it hard to track what changes were added since the last review. Don't mind a messy commit history in the pull request, we will squash & merge at the end anyway. |
3e48bb7
to
5deaacf
Compare
I usually prefer commit hygiene and not PR descriptions ending up as commit messages as usually this is not suitable, but well, fixed now to restore original commit and an additional with the changes. :-) |
action-types.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit concerned about the long-term maintenance of these types and requiring this action's maintainers to keep them up to date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, for the permissions I added the same update generation logic that is used for the action.yml
.
For the others, yes, you would have to maintain them.
Which imho is a good thing, as it makes clear to users which types the inputs have and which values are valid in a defined technical way,
and any automatic consumer like DSL-generators can leverage the information and it is always up-to-date with the state of your action.
Unless you constantly add and remove inputs or outputs (other than the permissions which are generated), there should not be much additional effort hopefully, and the added action makes sure you don't forget to update them and have a valid format.
And even if you very often change inputs / outputs, it is just adding it to that file too to define the type, so hopefully even then not a significant additional effort. :-)
I would like to see your action first-class supported by https://github.com/typesafegithub/github-workflows-kt, a Kotlin DSL to write GitHub Action workflows.
The project came up with ways to reduce operational load when keeping the library's action wrappers in sync with actions' inputs and outputs.
The solutions include onboarding https://github.com/typesafegithub/github-actions-typing.
It is as easy as adding an extra YAML file to your repository root, and (optionally) adding a simple GitHub workflow that validates this new file.
Thanks to this, the code generator in the Kotlin DSL can fetch typing info provided by you instead of them, which has a number of benefits.
It has no negative effects on current action consumers, they continue to use the action via regular GitHub API, as if the file was not there.
In this pull request, I would like to ask you if you are open to introduce such typings in your action and also provide the current state as far as I could determine it.
You would not be the first, there are already other actions using it, like e.g. my https://github.com/Vampire/setup-wsl or also Microsoft's https://github.com/microsoft/setup-msbuild.
Also "regular" users can benefit from this clear and formalized typing definition, seeing exactly what values are valid.
And as the typings are made independent from the Kotlin DSL library, also other DSL or similar consumers could use these typings in the future.
When accepting this PR and in the future maintaining the typing yourself,
please keep in mind that API are generated from these typing information.
So if you for example recognize that a type is wrong and want to change it for example from
string
tointeger
,this would be a breaking change for such consumers and should therefore be done with a major version bump.
Backwards compatible changes like new enum options, or new inputs or outputs can of course be released in a minor version as usual.
After hopefully merging this PR it would be nice if you could cut a release soon,
so that the shiny new typings can be used right away.