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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: assorted code readability improvements in h1/conn.rs #2817

Merged
merged 13 commits into from Apr 26, 2022
Merged

Refactor: assorted code readability improvements in h1/conn.rs #2817

merged 13 commits into from Apr 26, 2022

Conversation

BastiDood
Copy link
Contributor

Hello there! This PR features assorted refactoring techniques that hopefully improves readability. No behavior change is expected.

Most of the refactoring involved using the matches! macro instead of explicit Boolean match expressions. Other refactoring involved transforming logic to early returns. The overall result is that the code is now a little bit less nested. 馃帀

With that said, please feel free to inform me if some of the changes need improvement. Thanks!

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Thanks so much! This does indeed improve readability in various places. I did leave a comment inline, but I don't know if I want action on it or not... mostly just want to make a record of it...

src/proto/h1/conn.rs Outdated Show resolved Hide resolved
src/proto/h1/conn.rs Outdated Show resolved Hide resolved
src/proto/h1/conn.rs Outdated Show resolved Hide resolved
@BastiDood
Copy link
Contributor Author

Everything seems to be in order now. Please do let me know if there are any other concerns. Thanks!

@seanmonstar seanmonstar merged commit faf24c6 into hyperium:master Apr 26, 2022
17 of 18 checks passed
@seanmonstar
Copy link
Member

Very nice, thanks again!

@BastiDood BastiDood deleted the assorted-refactoring branch April 26, 2022 18:36
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.

None yet

2 participants