-
Notifications
You must be signed in to change notification settings - Fork 161
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
Linkify all class definitions #913
Conversation
@@ -3539,7 +3539,7 @@ lt="WritableStreamDefaultWriter(stream)">new WritableStreamDefaultWriter(<var>st | |||
1. Return *this*.[[closedPromise]]. | |||
</emu-alg> | |||
|
|||
<h5 id="default-writer-desiredSize" attribute for="WritableStreamDefaultWriter" lt="desiredSize">get desiredSize</h5> | |||
<h5 id="default-writer-desired-size" attribute for="WritableStreamDefaultWriter" lt="desiredSize">get desiredSize</h5> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to have consistent spelling with the other IDs and couldn’t find any other references to it. I am assuming I’m not breaking anything here?
@@ -1240,13 +1240,13 @@ would look like | |||
|
|||
<pre><code class="lang-javascript"> | |||
class ReadableStreamDefaultReader { | |||
constructor(stream) | |||
<a href="#default-reader-constructor">constructor(stream)</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is desirable for readability of the output, but it sure makes the source unreadable.
@tabatkins Can Bikeshed help our markup look nicer here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a different markup in #872 but the consensus was that simple a tags were preferred. I think these links could be automatically generated, so I am in favor of somehow using bikeshed for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we end up doing what @ricea suggested in #907 we'd align more with existing specs so I wouldn't suggest adding anything to Bikeshed for this definition.
That said, the domintro blocks (as proposed in #907) are also not very readable in the source either. See e.g. https://github.com/domenic/async-local-storage/blob/master/spec.bs#L239-L249 for my latest attempt. (Explicit <p>
s needed because of speced/bikeshed#1203.)
Personally I'm not too worried about the source readability here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, and sorry for the delay! Added CountQueuingStrategy, and now merging.
🎉🎉🎉 |
As a follow-up to #872, I linkified all the other class definitions as well.
Preview | Diff