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 AndroidEmulatorRunnerV2 #345

Closed
wants to merge 4 commits into from
Closed

Add AndroidEmulatorRunnerV2 #345

wants to merge 4 commits into from

Conversation

LeoColman
Copy link
Contributor

No description provided.

Copy link
Contributor

@jmfayard jmfayard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution
We have typings though to make things more type safe and right now they have to be deduced manually and put into wrapperstogenerate

@krzema12
Copy link
Member

krzema12 commented Jul 15, 2022

...right now they have to be deduced manually and put into wrapperstogenerate

@LeoColman unless you can ask the action owner to provide the typings by onboarding https://github.com/krzema12/github-actions-typing :) Then this PR would be literally a one-liner + generated code. It would be actually the preferred path forward, and if it fails/takes long, we can fall back to placing the typings here as you did.

LeoColman and others added 2 commits July 15, 2022 13:19
…wrappergenerator/WrappersToGenerate.kt

Co-authored-by: Piotr Krzemiński <3110813+krzema12@users.noreply.github.com>
@LeoColman LeoColman requested review from jmfayard and krzema12 and removed request for krzema12 July 15, 2022 18:22
Copy link
Member

@krzema12 krzema12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a failed build - ktlint complains, could you fix the style?
Also let me ensure that you've seen my recommendation to ask the owner to provide the typings in the action itself. I've documented it in the contribution guidelines: https://github.com/krzema12/github-actions-kotlin-dsl/blob/19783861f4ba2eddc28fc2f2085ba1f7ca3e7b45/CONTRIBUTING.md?plain=1#L18 this is non-blocking for this PR, but would be good to do it simultaneously 😄

@LeoColman
Copy link
Contributor Author

There's a failed build - ktlint complains, could you fix the style? Also let me ensure that you've seen my recommendation to ask the owner to provide the typings in the action itself. I've documented it in the contribution guidelines:

https://github.com/krzema12/github-actions-kotlin-dsl/blob/19783861f4ba2eddc28fc2f2085ba1f7ca3e7b45/CONTRIBUTING.md?plain=1#L18
this is non-blocking for this PR, but would be good to do it simultaneously smile

Ah, I thought it was more of an OR xD
Let me drop a PR to the action, it will be easier that way for the authors

@LeoColman
Copy link
Contributor Author

Complementary PR:
ReactiveCircus/android-emulator-runner#257

@LeoColman
Copy link
Contributor Author

Also, I've fixed the KTlint requirement.

@LeoColman LeoColman requested a review from krzema12 July 16, 2022 14:30
Copy link
Contributor

@jmfayard jmfayard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@krzema12
Copy link
Member

@LeoColman thanks for this! I'll take care of merging.

@krzema12
Copy link
Member

Merging in #357.

@krzema12 krzema12 closed this Jul 24, 2022
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