- From: Anne van Kesteren <notifications@github.com>
- Date: Mon, 07 Nov 2016 01:04:46 -0800
- To: whatwg/fetch <fetch@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/412/review/7374819@github.com>
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<sequence<ByteString>> or OpenEndedDictionary<ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre> - -<div class=XXX> - <p>OpenEndedDictionary<T> is a future IDL construct. Expect it to be used as such: - - <pre> -var meta = { "Content-Type": "text/xml", "Breaking-Bad": "<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<sequence<ByteString>> or record<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<sequence<ByteString>> or OpenEndedDictionary<ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre> - -<div class=XXX> - <p>OpenEndedDictionary<T> is a future IDL construct. Expect it to be used as such: - - <pre> -var meta = { "Content-Type": "text/xml", "Breaking-Bad": "<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