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

Rename errors #14

Merged
merged 1 commit into from
Mar 2, 2020
Merged

Rename errors #14

merged 1 commit into from
Mar 2, 2020

Conversation

Wondertan
Copy link
Contributor

@@ -5,69 +5,69 @@ import (
"fmt"
)

type YamuxError struct {
type Error struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously was started with repository suffix.

// if the backlog is exceeded, or if there was a remote GoAway.
ErrConnectionReset = &YamuxError{msg: "stream reset"}
ErrStreamReset = &Error{msg: "stream reset"}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a connection reset, but a stream one.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM but let's not merge this till we have fixes in go-libp2p-yamux. Usually, I'm not a fan of cosmetic breaking changes but I'm pretty sure the only use of these types/values is in go-libp2p-yamux.

@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Feb 28, 2020
@Wondertan
Copy link
Contributor Author

@Stebalien, what about merging this first and then pointing go-libp2p-yamux to merged updates here?

@Stebalien Stebalien merged commit ffc6806 into libp2p:master Mar 2, 2020
@Stebalien Stebalien removed the status/blocked Unable to be worked further until needs are met label Mar 2, 2020
@Stebalien
Copy link
Member

Done and released.

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

Successfully merging this pull request may close these issues.

2 participants