Reorganize Spec for consistency and clarity #8
Conversation
| similarities with that document. | ||
| </div> | ||
|
|
||
| <h2 id="concepts">Concepts</h2> |
There was a problem hiding this comment.
I am not sure we need this section. I like how it is in the Streams spec because it lays out the higher level concepts before diving into all the individual (and complex) classes. But this spec is simpler, so I think this is redundant. Should I remove this section and move the explanations into the various other relevant sections?
There was a problem hiding this comment.
Unless other folks find value in this section, I'm going to remove it. I just tried it out locally and I personally like it more that way. It simplifies the spec and declutters some of the linking nonesense.
There was a problem hiding this comment.
No strong feelings either way here.
fix all linking issues
Ethan-Arrowood
force-pushed
the
reorganize
branch
from
942a3f2
to
ae54381
Compare
| * the `close` method is called on the socket | ||
| * the socket was constructed with the `allowHalfOpen` parameter set to `false`, the ReadableStream | ||
| is being read from, and the remote connection sends a FIN packet (graceful closure) or a RST | ||
| packet | ||
| <ul> | ||
| <li>the {{close()}} method is called on the socket</li> | ||
| <li>the socket was constructed with the `allowHalfOpen` parameter set to `false`, the ReadableStream is being read from, and the remote connection sends a FIN packet (graceful closure) or a RST packet</li> | ||
| </ul> |
There was a problem hiding this comment.
Aren't these equivalent? just a style preference?
There was a problem hiding this comment.
Yes, just trying to be consistent. There was * - and <ol> used so I switch everything to HTML
| case where this can happen is if the input address is incorrectly formatted. | ||
| Errors which occur asynchronously- after the socket instance has been returned- must reject the | ||
| socket's `closed` promise. | ||
| At any point during the creation of the {{Socket}} instance, `connect` may throw a [=SocketError=]. One case where this can happen is if the input address is incorrectly formatted. Errors which occur asynchronously (after the socket instance has been returned) must reject the socket's {{closed}} promise. |
There was a problem hiding this comment.
Nitpicking but it would be nice to keep to a 80 or 100 line length limit. (there are cases above as well where the wrapping is missing too)
There was a problem hiding this comment.
Ahh yeah I tend to favor wrapping only if theres a tool to do it for me. Otherwise its easier to leave everything and toggle word wrap in the editor. Let me look into if theres an automation i can add for this (maybe just a vscode plugin or something)
There was a problem hiding this comment.
speced/bikeshed#662 (comment) :( so this wont exist in Bikeshed ever. A quick search also came up empty. My preference in this circumstance is to not wrap because then we'll have to manually check line lengths every time we make changes (and I guarantee we'll miss something). If you feel strongly I'm happy to manually wrap though 👍 doesn't matter that much to me
| similarities with that document. | ||
| </div> | ||
|
|
||
| <h2 id="concepts">Concepts</h2> |
There was a problem hiding this comment.
No strong feelings either way here.
Co-authored-by: Dominik Picheta <[email protected]>
Co-authored-by: Dominik Picheta <[email protected]>
Co-authored-by: Dominik Picheta <[email protected]>
Co-authored-by: Dominik Picheta <[email protected]>
|
Thank you for the review! I'm going to merge this now and then we can iterate on some of the other pieces in follow up PRs. Things to revisit:
|
While attempting to fix the problems detailed in #5 I realized that there was a lot of inconsistencies between the usage of markdown and html. Furthermore, the linking was so difficult for me to fix.
This PR is a rewrite/reorganization of the spec that follows patterns I see in the Streams and Fetch specs. It strictly uses HTML elements instead of markdown. It also attempts to reorganize the information in such a way that allows for easier linking, and ideally improved readability.
Still some minor things to fix up. But wanted to share so we can start reviewing asap.
🚀