Skip to content
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

Consistent event semantics and documentation #73

Merged
merged 4 commits into from
Mar 8, 2017

Conversation

clue
Copy link
Member

@clue clue commented Mar 7, 2017

Et voilà, consistent event semantics and documentation 🎉

This pretty much only adds consistent documentation for what is currently undocumented and implicitly assumed.

This should in fact not break any of the existing assumptions and as such should not be considered a BC break. Given that this current behavior was mostly underdocumented or undocumented, I'll mark this as a BC break just to be safe in case any implementation relies on undocumented features which might now be prohibited.

Builds on top of #72.

README.md Outdated
In other words, many lower-level protocols (such as TCP/IP) may choose
to only emit this for a fatal transmission error once and will thus
likely close (terminate) the stream in response.
It this is a fatal error that results in the stream being closed, it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this...

README.md Outdated
});
```

This event SHOULD be emitted once every time the buffer become full
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...became...(?)

README.md Outdated

The `pipe` event will be emitted whenever a readable stream is `pipe()`d
into this stream.
Event event receives a single `ReadableStreamInterface` argument for the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event...

README.md Outdated
only deal with data transmission and may choose
to only emit this for a fatal transmission error once and will thus
likely close (terminate) the stream in response.
It this is a fatal error that results in the stream being closed, it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this...

*
* This event SHOULD be emitted once or never at all, depending on whether
* the stream ever terminates.
* It SHOULD NOT be emitted afer a previous `close` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...emitted after a...

*
* This event SHOULD be emitted once or never at all, depending on whether
* the stream ever terminates.
* It SHOULD NOT be emitted afer a previous `close` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...emitted after a...

README.md Outdated

This event SHOULD be emitted once or never at all, depending on whether
the stream ever terminates.
It SHOULD NOT be emitted afer a previous `close` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...emitted after a...

README.md Outdated

This event SHOULD be emitted once or never at all, depending on whether
the stream ever terminates.
It SHOULD NOT be emitted afer a previous `close` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...emitted after a...

@clue clue force-pushed the event-semantics branch from a6eb097 to 42e8181 Compare March 7, 2017 10:53
@clue clue force-pushed the event-semantics branch from 42e8181 to 8af6086 Compare March 7, 2017 10:55
@clue
Copy link
Member Author

clue commented Mar 7, 2017

Thanks for spotting these, typos fixed and changes squashed :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants