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

Change name from target client id to replaces client id #3788

Merged
merged 2 commits into from
Aug 24, 2018

Conversation

jungkees
Copy link
Contributor

@jungkees jungkees commented Jul 1, 2018

We had decided to change the name of FetchEvent.targetClientId to
.replacesClientId to clarify the meaning that this client is a to be replaced
client: w3c/ServiceWorker#1091 (comment).
Accordingly, this changes the reference to the request's target client id to
request's replaces client id.

Related issue: w3c/ServiceWorker#1245.

SW PR: w3c/ServiceWorker#1333.
Fetch PR: whatwg/fetch#774.


/browsing-the-web.html ( diff )
/infrastructure.html ( diff )

We had decided to change the name of FetchEvent.targetClientId to
.replacesClientId to clarify the meaning that this client is a to be replaced
client: w3c/ServiceWorker#1091 (comment).
Accordingly, this changes the reference to the request's target client id to
request's replaces client id.

Related issue: w3c/ServiceWorker#1245.

SW PR: w3c/ServiceWorker#1333.
Fetch PR: whatwg/fetch#774.
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM. I'll mark "do not merge yet" until the Fetch dependency gets updated.

source Outdated
@@ -2832,7 +2832,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li><dfn data-x="concept-request-body" data-x-href="https://fetch.spec.whatwg.org/#concept-request-body">body</dfn></li>
<li><dfn data-x="concept-request-client" data-x-href="https://fetch.spec.whatwg.org/#concept-request-client">client</dfn></li>
<li><dfn data-x="concept-request-reserved-client" data-x-href="https://fetch.spec.whatwg.org/#concept-request-reserved-client">reserved client</dfn></li>
<li><dfn data-x="concept-request-target-client-id" data-x-href="https://fetch.spec.whatwg.org/#concept-request-target-client-id">target client id</dfn></li>
<li><dfn data-x="concept-request-target-client-id" data-x-href="https://fetch.spec.whatwg.org/#concept-request-replaces-client-id">replaces client id</dfn></li>
Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, need to update this data-x too, otherwise the build will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. I updated it.

@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label Jul 2, 2018
annevk pushed a commit to whatwg/fetch that referenced this pull request Aug 24, 2018
We had decided to change the name of FetchEvent's targetClientId to replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the name of request's target client id to replaces client id.

See also: 

* w3c/ServiceWorker#1245
* w3c/ServiceWorker#1333
* whatwg/html#3788
@annevk annevk removed the do not merge yet Pull request must not be merged per rationale in comment label Aug 24, 2018
@annevk annevk merged commit 951f9b0 into whatwg:master Aug 24, 2018
jungkees added a commit to w3c/ServiceWorker that referenced this pull request Aug 26, 2018
We had decided (and I forgot) to change the name of FetchEvent.targetClientId to
.replacesClientId to clarify the meaning that this client is a to-be-replaced
client: #1091 (comment).

Fetch PR: whatwg/fetch#774.
HTML PR: whatwg/html#3788.

Related issue: #1245.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants