fix: allow api level >= 35 instead of ==35 for android platformSetupCheck#154
Conversation
There was a problem hiding this comment.
created a separate android specific util class to hold these android unique methods: easy testing and isolation.
| ): { | ||
| allRequirementsMet: boolean; | ||
| errorMessages: string[]; | ||
| tests: PlatformCheckResult['tests']; |
There was a problem hiding this comment.
pass back the tests from the passed json.
| } | ||
|
|
||
| const versionAtLeast = (pkg: { version: Version | string }) => { | ||
| if (typeof pkg.version === 'string') { |
There was a problem hiding this comment.
Do we need to check version's type? Version.sameOrNewer has the first parameter as Version and string type already.
| * for Android and replace with a direct >= apiLevel check (platform + emulator image). | ||
| * Values must match messages in @salesforce/lwc-dev-mobile-core (requirement-android.md). | ||
| */ | ||
| /** Titles of setup requirement checks we skip for Android (replaced by direct >= apiLevel check). */ |
There was a problem hiding this comment.
We should consolidate the above 2 doc comments into one
| */ | ||
| /** Titles of setup requirement checks we skip for Android (replaced by direct >= apiLevel check). */ | ||
| export const ANDROID_SETUP_REQUIREMENT_TITLES_TO_SKIP: readonly string[] = [ | ||
| 'Checking SDK Platform API', |
There was a problem hiding this comment.
Can we add a comment referencing where these titles come from in case upstream package changes its message text?
| if (!hasPlatformAtLeast) { | ||
| return { | ||
| success: false, | ||
| errorMessage: `Android platform API for level ${apiLevel} or higher are required. No installed platform package found with version >= ${apiLevel}.`, |
There was a problem hiding this comment.
nit: Should be 'is required'
haifeng-li-at-salesforce
left a comment
There was a problem hiding this comment.
LGTM. Great improvement!
c2275d3
into
forcedotcom:main
platformSetupCheck for android is strictly checking the api/emulator level 35 is installed. It should be >= 35.