Re: [whatwg/streams] Add more cross-links (#872)

domenic requested changes on this pull request.

Some good stuff here, thanks!

In addition to specific issues noted, let's be sure the final patch rewraps to the 120-character column limit.

> @@ -95,7 +95,7 @@ A <dfn export>readable stream</dfn> represents a source of data, from which you
 <em>out</em> of a readable stream. Concretely, a readable stream is an instance of the {{ReadableStream}} class.
 
 Although a readable stream can be created with arbitrary behavior, most readable streams wrap a lower-level I/O source,
-called the <dfn>underlying source</dfn>. There are two types of underlying source: push sources and pull sources.
+called the <dfn>underlying source</dfn> ([[#rs-constructor|interface description]]). There are two types of underlying source: push sources and pull sources.

This isn't good, because we're currently talking about the general concept of underlying sources, not the JS objects that sometimes (for web-developer-created streams) embody them. Such a link belongs later, where we say "For web developer–created streams, the implementation details of a source are provided by an object with certain methods and properties that is passed to the ReadableStream() constructor." But, that already links to the constructor, so I'm not sure if there's anything to do here.

> @@ -137,7 +137,7 @@ A <dfn export>writable stream</dfn> represents a destination for data, into whic
 goes <em>in</em> to a writable stream. Concretely, a writable stream is an instance of the {{WritableStream}} class.
 
 Analogously to readable streams, most writable streams wrap a lower-level I/O sink, called the
-<dfn>underlying sink</dfn>. Writable streams work to abstract away some of the complexity of the underlying sink, by
+<dfn>underlying sink</dfn> ([[#ws-constructor|interface description]]). Writable streams work to abstract away some of the complexity of the underlying sink, by

Same problem as above.

> @@ -385,15 +385,15 @@ like
 
 <pre><code class="lang-javascript">
   class ReadableStream {
-    constructor(underlyingSource = {}, { size, highWaterMark } = {})
+    <l>[[#rs-constructor|constructor]]</l>(underlyingSource = {}, { size, highWaterMark } = {})

Personally I find this more confusing than just doing `<a href="#rs-constructor">constructor</a>`. Does that work, or is only `<l>` allowed inside `<pre>` blocks?

> @@ -444,11 +444,11 @@ ReadableStream(<var>underlyingSource</var> = {}, { <var>size</var>, <var>highWat
   govern how the constructed stream instance behaves:
 
   <ul>
-    <li><p><code>start(controller)</code> is called immediately, and is typically used to adapt a <a>push
+    <li><p><code>start({{ReadableStreamDefaultController|controller}})</code> is called immediately, and is typically used to adapt a <a>push

I'm not really OK with linking variable names to their types in general, here or elsewhere. (Especially, as you say, when their types are not accurate.) If you think it's unclear what the type of the controller is, we should add a more explicit sentence explaining.

This occurs several times below; I'll avoid noting it each time.

> @@ -385,15 +385,15 @@ like
 
 <pre><code class="lang-javascript">

The introduction of links into these blocks is a great improvement; thank you!

-- 
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/streams/pull/872#pullrequestreview-92017082

Received on Saturday, 27 January 2018 04:52:48 UTC