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

use DeviceCodeName for code name and DeviceName for name #7

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

tony2001
Copy link
Contributor

they are actually pretty different in some cases and both have useful
information.

typical example:
Device_Name="Galaxy S5 LTE"
Device_Code_Name="SM-G900F"

they are actually pretty different in some cases and both have useful
information.

typical example:
Device_Name="Galaxy S5 LTE"
Device_Code_Name="SM-G900F"
}

func (browser *Browser) IsIPad() bool {
return browser.Platform == "iOS" && browser.DeviceName == "iPad"
return browser.Platform == "iOS" && browser.DeviceCodeName == "iPad"

Choose a reason for hiding this comment

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

typical example:
Device_Name="Galaxy S5 LTE"
Device_Code_Name="SM-G900F"

Shouldn't this line still be:
return browser.Platform == "iOS" && browser.DeviceName == "iPad"

@@ -214,11 +220,11 @@ func (browser *Browser) IsAndroid() bool {
}

func (browser *Browser) IsIPhone() bool {
return browser.Platform == "iOS" && browser.DeviceName == "iPhone"
return browser.Platform == "iOS" && browser.DeviceCodeName == "iPhone"

Choose a reason for hiding this comment

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

Same as below, I think this should still be
return browser.Platform == "iOS" && browser.DeviceName == "iPhone"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is there because browser.DeviceName used to take value from DeviceCodeName line in the INI file (which I believe is wrong and misleading). For iPhone and iPad these lines seem to be the same, but I don't believe it's correct to expect that DeviceName should always match DeviceCodeName since they are technically different things.

Copy link
Contributor

Choose a reason for hiding this comment

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

My take is that DeviceName is more semantically accurate here. (I don't think "iPhone" is a code-name anymore :-). ) As you point out, in the current INI file the device-name and device-code-name area always the same for iPhone and iPad, so it doesn't make a difference as far as the code is concerned, it's just which makes more semantic sense.

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.

4 participants