fix: allow google_apis_ps16k as a valid target#440
fix: allow google_apis_ps16k as a valid target#440ychescale9 merged 3 commits intoReactiveCircus:mainfrom
Conversation
|
Saw in related comment here #424 (comment) that needed to add to test and do the transpile to JS, so just pushed that |
ychescale9
left a comment
ychescale9
left a comment
There was a problem hiding this comment.
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 |
ychescale9
left a comment
ychescale9
left a comment
There was a problem hiding this comment.
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? |
|
@ychescale9 any chance of a release with this PR in it 🙏 ? I tried to integrate it into our CI via the commit SHA but it failed for an unexpected reason |
|
Sorry for the delay! Will try to do a release today. |
* main: Prepare for release 2.35.0. docs: update AVD profile description (#452) README: Fix imbalanced backtick in `Configurations` table (#445) fix: allow google_apis_ps16k as a valid target (#440) Fix `pre-emulator-launch-script` (#439) Optimize config.ini updates and efficiency improvements report (#436) Fix outdated information about larger runners billing (#437)
3 participants
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!