-
-
Notifications
You must be signed in to change notification settings - Fork 227
fix: allow google_apis_ps16k as a valid target #440
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
Conversation
Saw in related comment here #424 (comment) that needed to add to test and do the transpile to JS, so just pushed that |
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.
Thanks for the contribution! If you don't mind can you please update the table in README.md
to include google_apis_ps16k
in the description
?
these appear to have too much maintenance burden as they only ever add more over time
I addressed the thought on target validation with a redone implementation that removes validation but still allows (and tested) the playstore shorthand I updated the docs related to target to reflect current reality of targets based on |
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.
should we also update actions.yml
's target description to:
target:
description: 'target of the system image - e.g. `default`, `google_apis`, `google_apis_ps16k`, `playstore`, `playstore_ps16k`, `android-wear`, `android-wear-cn`, `android-tv`, `google-tv`, `aosp_atd`, `google_atd`, `android-automotive`, `android-automotive-playstore, `android-desktop`. Please run `sdkmanager --list` to see the available targets.'
… targets Fixes ReactiveCircus#403 Co-authored-by: Yang <reactivecircus@gmail.com>
…ill work no longer requires code changes here to access new targets
I found the 'playstore' (and 'playstore_ps16k') shortcuts to be surprising personally, in all other cases you just use the actual target name but there are these two underdocumented shortcuts? So I did update action.yml and README but I used the real sdkmanager target names so that people just get used to using those I did not remove that feature though, I respect backwards-compability a lot more than that - just for documentation seems using the actual target names is more consistent I pushed an updated second commit with the README.md and action.yml changes what do you think? |
Looks good! |
* test: remove target validator test these appear to have too much maintenance burden as they only ever add more over time * fix: allow google_apis_ps16k and google_apis_playstore_ps16k as valid targets Fixes ReactiveCircus#403 Co-authored-by: Yang <reactivecircus@gmail.com> * fix: remove target validation entirely, any valid sdkmanager target will work no longer requires code changes here to access new targets --------- Co-authored-by: Yang <reactivecircus@gmail.com>
Can we have a tagged version with this change? |
Hi there, we'd like to test using the new 16kb page size to make sure things work, but isn't working with the action here currently
As pointed out in the linked issue, it's a fairly trivial input validator change, so I thought I'd just propose a PR
Thanks!