Summary: | Standardize device/simulator naming conventions | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Bedard <jbedard> | ||||||||||||||||||
Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | ap, commit-queue, dbates, glenn, lforschler, webkit-bug-importer, webkit | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||
Bug Blocks: | 169302 | ||||||||||||||||||||
Attachments: |
|
Description
Jonathan Bedard
2017-03-02 10:06:13 PST
Created attachment 303215 [details]
Patch
Looks good to me. 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. Created attachment 303218 [details]
Patch
Created attachment 303220 [details]
Patch
Comment on attachment 303220 [details]
Patch
LGTM
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. Created attachment 303681 [details]
Patch
Created attachment 303682 [details]
Patch
Created attachment 303715 [details]
Patch
Created attachment 303721 [details]
Patch
Comment on attachment 303721 [details] Patch Clearing flags on attachment: 303721 Committed r213545: <http://trac.webkit.org/changeset/213545> All reviewed patches have been landed. Closing bug. Reverted r213545 for reason: iOS EWS broken by this change Committed r213577: <http://trac.webkit.org/changeset/213577> Created attachment 303857 [details]
Patch
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. 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. > 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.
Comment on attachment 303857 [details] Patch Clearing flags on attachment: 303857 Committed r213654: <http://trac.webkit.org/changeset/213654> All reviewed patches have been landed. Closing bug. |