Re: [whatwg/fetch] Use Web IDL's new record type (#412)

annevk requested changes on this pull request.

Overall this looks great. Thank you for working on this. I have a couple nits and we need some tooling changes that I requested here: https://github.com/plinss/widlparser/issues/18.

I'd also appreciate it if you could add your name to the Acknowledgments section, but no worries if you don't want to.

> @@ -4039,17 +4039,7 @@ fetch("/music/pk/altes-kamuffel.flac")
 
 <h3 id=headers-class>Headers class</h3>
 
-<pre data-no-idl class=idl>typedef (Headers or sequence&lt;sequence&lt;ByteString>> or OpenEndedDictionary&lt;ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre>
-
-<div class=XXX>
- <p>OpenEndedDictionary&lt;T> is a future IDL construct. Expect it to be used as such:
-
- <pre>
-var meta = { "Content-Type": "text/xml", "Breaking-Bad": "&lt;3" }
-new Headers(meta)</pre>
-
- <p>See <a href=https://github.com/whatwg/fetch/issues/164>issue #164</a> for discussion.
-</div>
+<pre data-no-idl class=idl>typedef (Headers or sequence&lt;sequence&lt;ByteString>> or record&lt;ByteString, ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre>

The "data-no-idl" can be removed now this is all part of IDL.

> @@ -4039,17 +4039,7 @@ fetch("/music/pk/altes-kamuffel.flac")
 
 <h3 id=headers-class>Headers class</h3>
 
-<pre data-no-idl class=idl>typedef (Headers or sequence&lt;sequence&lt;ByteString>> or OpenEndedDictionary&lt;ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre>
-
-<div class=XXX>
- <p>OpenEndedDictionary&lt;T> is a future IDL construct. Expect it to be used as such:
-
- <pre>
-var meta = { "Content-Type": "text/xml", "Breaking-Bad": "&lt;3" }
-new Headers(meta)</pre>

I would prefer to keep this example. Change the class on the `<div>` to `example` and change the opening paragraph to something like: `Constructing a {{Headers}} object is fairly straightforward:`

>     <li><p><a lt=append for=Headers>Append</a>
-   <var>header</var>'s key/<var>header</var>'s value to
+   <var>key</var>/<var>value</var> to
    <var>headers</var>. Rethrow any exception.

Since this is now the only substep you should just inline it in the paragraph that introduces it. "then for each mapping ... in object, append ..."

>  
   <ol>
-   <li><p>Set <var>header</var>'s key to <var>header</var>'s key,
-   <a spec=webidl>converted to ByteString</a>.

Since this is removed we can remove the corresponding line in `class=anchors`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/412#pullrequestreview-7374819

Received on Monday, 7 November 2016 09:05:17 UTC