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

Use "closed" instead of "disconnected" #561

Closed
wants to merge 1 commit into from
Closed

Use "closed" instead of "disconnected" #561

wants to merge 1 commit into from

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Sep 7, 2020

Addresses #256 (comment)

@ghost
Copy link

ghost commented Sep 7, 2020

My main worry about this change is that it would break too much existing code for very little benefit. To see if that is true, I cloned the Servo repository and grepped it for 'disconnected'. I only found one instance of breakage:

https://github.com/servo/servo/blob/a5a21a59addae0df6d9e050f17d44399db04fec3/components/script/dom/window.rs#L1735

This makes me feel better considering that Servo is a big codebase. Still... I'm a bit worried that other projects might be matching on error types more often. Crossbeam might have reached the point where it's so widespread that stability is preferable over minor API improvements.

What do you think?

@taiki-e
Copy link
Member Author

taiki-e commented Sep 7, 2020

Hmm, given that the current term is not wrong, the cost of changing this will probably be greater than the benefit. (I found nearly 50 "Try(Send|Recv)Error::Disconnected" in tikv.)

What I was thinking was mainly about the impact on methods that may be added in the future, such as disconnect/close and is_disconnected/is_closed. (As time goes by, it becomes more difficult to standardize terms within the ecosystem. But in reality, it may already be impossible...)

@taiki-e taiki-e mentioned this pull request Sep 7, 2020
@ghost
Copy link

ghost commented Sep 7, 2020

Yeah I think that ship has sailed :) We'll probably never agree on a 'standard' channel interface, but that's okay. I don't think it's a super important problem worth solving. We're fine with a bunch similar but slightly different interfaces...

@taiki-e
Copy link
Member Author

taiki-e commented Sep 7, 2020

Ok! Closing in favor of the above opinion that the benefits of doing this are not enough.

@taiki-e taiki-e closed this Sep 7, 2020
@taiki-e taiki-e deleted the close branch September 7, 2020 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant