| Summary: | WebKitTestRunner: Add watchOS entitlements | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jonathan Bedard <jbedard> | ||||
| Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, dbates, lforschler, thorton, webkit-bug-importer, wenson_hsieh | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Jonathan Bedard
2018-08-20 14:28:34 PDT
Created attachment 347547 [details]
Patch
Talking with Tim Horton, while service workers are not supported on watchOS, in the event that they were supported in the future, someone would need to re-discover this entitlement, so I kept it. Comment on attachment 347547 [details] Patch Clearing flags on attachment: 347547 Committed r235099: <https://trac.webkit.org/changeset/235099> All reviewed patches have been landed. Closing bug. (In reply to Jonathan Bedard from comment #2) > Talking with Tim Horton, while service workers are not supported on watchOS, > in the event that they were supported in the future, someone would need to > re-discover this entitlement, so I kept it. We (the WebKit OpenSource Project and Apple) practice <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add functionality until deemed necessary. And this bug does just that. Adding "dead code" like what is being done in this patch hurts hackability and I suggest we revert <https://trac.webkit.org/changeset/235099>. The entitlement can be found by looking at <https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/Configurations/WebKitTestRunnerApp-iOS.entitlements?rev=228589> or grepping ChangeLogs and blaming code to find the code changes associated with the bug #182865 , which added the entitlement. (In reply to Daniel Bates from comment #6) > (In reply to Jonathan Bedard from comment #2) > > Talking with Tim Horton, while service workers are not supported on watchOS, > > in the event that they were supported in the future, someone would need to > > re-discover this entitlement, so I kept it. > > We (the WebKit OpenSource Project and Apple) practice > <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add > functionality until deemed necessary. And this bug does just that. *And this bug does does the opposite: It adds functionality that is not necessary. (In reply to Daniel Bates from comment #6) > (In reply to Jonathan Bedard from comment #2) > > Talking with Tim Horton, while service workers are not supported on watchOS, > > in the event that they were supported in the future, someone would need to > > re-discover this entitlement, so I kept it. > > We (the WebKit OpenSource Project and Apple) practice > <https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it>: do not add > functionality until deemed necessary. And this bug does just that. Adding > "dead code" like what is being done in this patch hurts hackability and I > suggest we revert <https://trac.webkit.org/changeset/235099>. The > entitlement can be found by looking at > <https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/ > Configurations/WebKitTestRunnerApp-iOS.entitlements?rev=228589> or grepping > ChangeLogs and blaming code to find the code changes associated with the bug > #182865 , which added the entitlement. Do not revert <https://trac.webkit.org/changeset/235099>. There are two entitlements in that change, one which is most certainly being used. If you would like to remove the Service Workers entitlement, then upload a patch to do so. |