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

Add typings and validation workflow #257

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

krzema12
Copy link
Contributor

@krzema12 krzema12 commented Aug 1, 2024

Add typings using https://github.com/typesafegithub/github-actions-typing, so that other tools can make use of them.

@ktrz
Copy link
Member

ktrz commented Aug 1, 2024

Cześć @krzema12

Thanks for your contribution! I've looked into your typing action and I'm not fully sold on it yet. Could you explain what are the benefits of it? For now it looks like another place to remember to update types definitions as I still need to maintain TS types (especially for enums).

Would it be possible to just try and use the existing TS types if possible (I could modify how I structure the types if necessary) Let me know :)

I'll be back in PL tomorrow night (and in GDA over the weekend) if you would like to discuss

@krzema12
Copy link
Contributor Author

krzema12 commented Aug 1, 2024

Hi @ktrz!

The goal here is to have a programming language-agnostic, easy-to-parse typings in a standardized format. This format is already used by a bunch of actions. For now the only tool I know that makes use of these typings is https://github.com/typesafegithub/github-workflows-kt. For actions that don't yet provide typings, there's https://github.com/typesafegithub/github-actions-typing-catalog (also for this action: https://github.com/typesafegithub/github-actions-typing-catalog/blob/main/typings/benchmark-action/github-action-benchmark/v1/action-types.yml).

Why not TS typings? Because the TS layer is merely an implementation detail using a particular programming language, and it's challenging to parse these in a generic and scalable way because one can implement action's logic even in some exotic languages.

What you could try to do to make things easier to maintain is to have a piece of logic that asserts on equivalence of TS types and action-typing.yml, or maybe auto-commits updates to the YAML based on changes in TS. Think of it as exporting action's types in a standardized form.

What a coincidence that you'll visit Gdansk 😊 This time I won't manage to meet, unfortunately.

@krzema12
Copy link
Contributor Author

krzema12 commented Aug 1, 2024

Fun fact: even Microsoft uses it 😎 https://github.com/microsoft/setup-msbuild/blob/main/action-types.yml Just without the validating action.

@krzema12
Copy link
Contributor Author

Hi @ktrz, let's resume our discussion here, I'm wondering that you think about what I wrote.

@ktrz
Copy link
Member

ktrz commented Sep 4, 2024

Hey @krzema12
I'm all for having a standardized format for action types!
My reservation is that I couldn't find any mention of how it could be used from a consumer point of view (ie. validating the calling workflow that it uses correct props). Are you planning to add such functionalities, or maybe I missed something and it already exists?

Additionally, as far as I understand it is meant to be used alongside a released action. In the case of this action, we build and push the action to a different branch --> v1. I think it would make more sense to also include it in the output somewhere alongside the build version, right?

@krzema12
Copy link
Contributor Author

krzema12 commented Sep 4, 2024

My reservation is that I couldn't find any mention of how it could be used from a consumer point of view (ie. validating the calling workflow that it uses correct props). Are you planning to add such functionalities, or maybe I missed something and it already exists?

So far the only consumer of the typings I'm aware of is https://github.com/typesafegithub/github-workflows-kt, and thanks to the typings we get type-safety when using actions from Kotlin. I'm not aware of any tool that would perform validation if one uses a YAML-based workflow, and I think it would be a good idea to create such tool, but for now it's beyond the scope of what I'm planning to work on since right now I'm focused on the Kotlin DSL way of consuming the actions. I hope that someone from the community will see a potential in this approach and will add some validation facilities to the YAML approach, or maybe even GitHub will consider adding it as a first-party feature.

Additionally, as far as I understand it is meant to be used alongside a released action. In the case of this action, we build and push the action to a different branch --> v1. I think it would make more sense to also include it in the output somewhere alongside the build version, right?

Yes, that's right. The typings should be co-located with action.yml, and indeed you'll likely need to adjust your release process.

@ktrz
Copy link
Member

ktrz commented Sep 9, 2024

Ok @krzema12
I've added one comment but I'll approve and merge when we resolve that one :) I'll do a follow-up to include that in the release script as well

@krzema12 krzema12 requested a review from ktrz September 9, 2024 10:14
@ktrz
Copy link
Member

ktrz commented Sep 9, 2024

@krzema12 could you please run npm run format and push the changes? I tried to update your branch but for some reason it didn't push properly 🤔

@krzema12 krzema12 requested a review from ktrz September 9, 2024 11:33
@ktrz ktrz merged commit 9c9d03a into benchmark-action:master Sep 9, 2024
27 checks passed
@krzema12 krzema12 deleted the add-typings branch September 9, 2024 11:46
@ktrz
Copy link
Member

ktrz commented Sep 9, 2024

thank you @krzema12 for your contribution!

@krzema12
Copy link
Contributor Author

krzema12 commented Sep 9, 2024

@ktrz thanks as well! Really hoping this standard gets more and more adoption.

BTW, are you planning to release a version with the typings somehow soon?

@ktrz
Copy link
Member

ktrz commented Sep 9, 2024

@ktrz thanks as well! Really hoping this standard gets more and more adoption.

BTW, are you planning to release a version with the typings somehow soon?

I'll release it in the coming days. Need to check other outstanding PRs. Will let you know

@Vampire
Copy link

Vampire commented Sep 9, 2024

@ktrz maybe a good idea for another project from you :-D
Providing some utility that generates parsing / validation code from the typings file as TypeScript sources, so that you just define the typings in this format and then some code generator will generate for an action using the utility TS code that has the enums, does validation, and gives the action author a ready-made typed object to work with. :-)
(I'm not too fond of / familiar with TypeScript, so I will not create such a thing)

@krzema12
Copy link
Contributor Author

krzema12 commented Sep 9, 2024

☝️ I'd be happy to host such tool under the typesafegithub org!

@krzema12
Copy link
Contributor Author

@ktrz thanks as well! Really hoping this standard gets more and more adoption.
BTW, are you planning to release a version with the typings somehow soon?

I'll release it in the coming days. Need to check other outstanding PRs. Will let you know

Friendly ping - would you be able to release?

@ktrz
Copy link
Member

ktrz commented Oct 23, 2024

released in v1.20.4

CC: @krzema12

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