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

Switch to JS Action #163

Closed
wants to merge 0 commits into from
Closed

Switch to JS Action #163

wants to merge 0 commits into from

Conversation

joh-klein
Copy link
Contributor

@joh-klein joh-klein commented Jun 26, 2024

In this PR I changed the action to a "JavaScript action" (as I offered in #158) - this will make it easier to handle additional inputs.
I tried to keep the change as small as possible and not add any additional ideas.

What I haven't tried is the release/publish step.

@joh-klein
Copy link
Contributor Author

If you want me to I could use more of the GitHub provided TypeScript-Action-Template

@joh-klein
Copy link
Contributor Author

Anything I can do to move this along?

@mandarini
Copy link
Member

mandarini commented Sep 27, 2024

Hi @joh-klein ! I merged your other PR , THANK you!

As for this PR, this looks like quite a big change, and may be disruptive to some ppl that are using this package? Even though I understand you have kept the functionality intact, it seems like lots of changes.

What is the main reason you think we should switch to a JS Action? What limitations do you think the current setup has? I know I asked you to create a PR back in June, but on second thoughts, and seeing the PR, I am not sure I can justify the switch. I do hope you understand my concerns, and be sure that I 100% appreciate the time you put into this.

Is the motivation for the change just the easier way to get the inputs? Or is there more things as well that this change would serve?

@meeroslav
Copy link
Collaborator

Hi, the this original PR had too many changes so I recreated a new one here still using your original commits: #177

Once we cleanup the open PRs we can move find-successful-workflow.ts to src/main.ts but let's keep these separate.

Thank you for the PR

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.

4 participants