Bug 169083 – Standardize device/simulator naming conventions

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jonathan Bedard]

Currently, we use a different naming convention for device and simulator when command line flags are passed.  These ports should be using the same naming convention.  For example, if --ios-simulator is a valid command line flag, so should --ios-device.  If --device is a valid command line flag, so should --simulator.  These command line flags and port names should follow a more predictable pattern.

[Jonathan Bedard]

Created attachment 303215 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/30810466>

[Srinivasan Vijayaraghavan]

Looks good to me.

[Srinivasan Vijayaraghavan]

You'll also need to make changes to the QueueStatusServer since the name of the ews queue will now be ios-device-ews.

Also, it would be great more people on the operations team had reviewer privileges.

[Jonathan Bedard]

Created attachment 303218 [details]
Patch

[Jonathan Bedard]

Created attachment 303220 [details]
Patch

[Dean Johnson]

Comment on attachment 303220 [details]
Patch

LGTM

[Alexey Proskuryakov]

Comment on attachment 303220 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303220&action=review

> Tools/QueueStatusServer/config/queues.py:37
> -    "ios-ews",
> +    "ios-device-ews",

Not sure if we need this change. The queue name doesn't document what the queue does very well anyway (it's iOS Device armv7s build only), and the shorter it is, the less space it takes in Bugzilla UI.

> Tools/Scripts/build-webkit:96
> +  --ios-device  or --device         Use "iphoneos.internal" SDK if installed, else "iphoneos" SDK (iOS only)
> +  --ios-simulator or --simulator    Use the current iphonesimulator SDK (iOS only)

I think that it may be slightly nicer to put each option on its own line, so that it's easier to point out which ones are deprecated.

[Jonathan Bedard]

Created attachment 303681 [details]
Patch

[Jonathan Bedard]

Created attachment 303682 [details]
Patch

[Jonathan Bedard]

Created attachment 303715 [details]
Patch

[Jonathan Bedard]

Created attachment 303721 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 303721 [details]
Patch

Clearing flags on attachment: 303721

Committed r213545: <http://trac.webkit.org/changeset/213545>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jonathan Bedard]

Reverted r213545 for reason:

iOS EWS broken by this change

Committed r213577: <http://trac.webkit.org/changeset/213577>

[Jonathan Bedard]

Created attachment 303857 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 303857 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303857&action=review

> Tools/Scripts/webkitpy/common/config/ews.json:16
> +        "name": "ios-ews",

So this is the additional fix. That's surprising to me - I thought that the name was only used for display purposes.

r=me if this fixes the problem, but I'd like to know more about why it does.

[Jonathan Bedard]

Comment on attachment 303857 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303857&action=review

>> Tools/Scripts/webkitpy/common/config/ews.json:16
>> +        "name": "ios-ews",
> 
> So this is the additional fix. That's surprising to me - I thought that the name was only used for display purposes.
> 
> r=me if this fixes the problem, but I'd like to know more about why it does.

We actually pull the commands to run EWS from this JSON file, as I mention in https://bugs.webkit.org/show_bug.cgi?id=169385.

[Alexey Proskuryakov]

> We actually pull the commands to run EWS from this JSON file

This is correct. What surprises me is that we use the name, as most EWSes don't even have in specified in ews.json.

[WebKit Commit Bot]

Comment on attachment 303857 [details]
Patch

Clearing flags on attachment: 303857

Committed r213654: <http://trac.webkit.org/changeset/213654>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.