-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
feat: Add a possibility to pass a letter as a platformVersion #607
Conversation
partialMatchCandidate = {[device.udid]: deviceOS}; | ||
} | ||
if ((!_.includes(opts.platformVersion, '.') && platformVersion.major === deviceOS.major | ||
|| platformVersion.major === deviceOS.major && platformVersion.minor === deviceOS.minor) |
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 there be some extra parentheses in here? I'm trying to parse this condition and it is confusing.
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.
no parentheses needed, AND has priority over OR. The meaning of this particular expression is:
- if we don't have a dot in platformVersion then only compare major numbers
- otherwise compare both major and minor version numbers
- if any of the conditions above is truthy then bump our candidate if needed ( e.g. the current candidate is not assigned yet or its version is older than the current deviceOS version)
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.
I tested this with Android R preview emulator and build tool version 29
if (util.compareVersions(deviceOS, '==', platformVersion)) { | ||
const bothVersionsCanBeCoerced = semver.valid(deviceOS) && semver.valid(platformVersion); | ||
const bothVersionsAreStrings = _.isString(deviceOS) && _.isString(platformVersion); | ||
if (bothVersionsCanBeCoerced && deviceOS.version === platformVersion.version |
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.
👍
appium/appium#13983