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.

Sign up for GitHub

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

Jump to bottom

Require Output = () on WebSocketStream::on_upgrade #636

Closed
SabrinaJewson opened this issue Dec 17, 2021 · 2 comments
Closed

Require Output = () on WebSocketStream::on_upgrade #636

SabrinaJewson opened this issue Dec 17, 2021 · 2 comments
Labels
A-axum C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone
0.5

Comments

@SabrinaJewson
Copy link
Contributor

Feature Request

Motivation

By accepting a future with any return type, users can easily make a mistake if they for example give a future that outputs a Result, not realizing that the Result will simply be ignored.

Proposal

Change the bound on WebSocketUpgrade::on_upgrade to Fut: Future<Output = ()> in 0.5.
@davidpdrsn
Copy link
Member

Makes sense 👍

@davidpdrsn davidpdrsn added this to the 0.5 milestone Dec 17, 2021
@davidpdrsn davidpdrsn added A-axum C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue. labels Dec 17, 2021
davidpdrsn added a commit that referenced this issue Dec 21, 2021
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade
9b304e3 
Fixes #636
@davidpdrsn davidpdrsn mentioned this issue Dec 21, 2021
Merged
davidpdrsn added a commit that referenced this issue Dec 21, 2021
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade
1d19d90 
Fixes #636
davidpdrsn added a commit that referenced this issue Dec 21, 2021
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
7bac03b 
Fixes #636
@davidpdrsn
Copy link
Member

Fixed in #646

davidpdrsn added a commit that referenced this issue Dec 27, 2021
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
b9e4c5e 
Fixes #636
davidpdrsn added a commit that referenced this issue Jan 3, 2022
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
75e8a76 
Fixes #636
davidpdrsn added a commit that referenced this issue Jan 3, 2022
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
b7e48e6 
Fixes #636
davidpdrsn added a commit that referenced this issue Jan 23, 2022
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
8ed03e2 
Fixes #636
davidpdrsn added a commit that referenced this issue Jan 23, 2022
@davidpdrsn
Require Output = () on WebSocketStream::on_upgrade (#646)
5d79c63 
Fixes #636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-axum C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Milestone
0.5
Development

No branches or pull requests
2 participants
@davidpdrsn @SabrinaJewson