Rename Channel.bytesBeforeUnwritable() to writableBytes() #12530
Merged
normanmaurer merged 4 commits intomainfrom Jul 3, 2022
Merged
Rename Channel.bytesBeforeUnwritable() to writableBytes() #12530normanmaurer merged 4 commits intomainfrom
normanmaurer merged 4 commits intomainfrom
Conversation
Member
Author
|
This depends on #12517 |
chrisvest
approved these changes
Jun 29, 2022
Member
chrisvest
left a comment
chrisvest
left a comment
There was a problem hiding this comment.
Spotted a copy-paste boo in the javadoc. Otherwise looks good.
normanmaurer
force-pushed
the
writability
branch
2 times, most recently
from
475029b to
bdc6910
Compare
normanmaurer
force-pushed
the
writable_method
branch
from
93678a0 to
317af62
Compare
|
nit: typo in summary (bytesBeforeUnwritabled) |
Member
Author
Good catch! |
trustin
reviewed
Jun 30, 2022
Member
trustin
left a comment
trustin
left a comment
There was a problem hiding this comment.
Could Channel.isWritable() be left as a shortcut for writableBytes() > 0? What's the reasoning behind removing it?
trustin
changed the title
Member
Author
|
@trustin it feels redundant... that said if you are strong about it we could also make it a default method :) WDYT ? |
thinkerou
reviewed
Jun 30, 2022
normanmaurer
force-pushed
the
writability
branch
3 times, most recently
from
2e23aaa to
e3f118c
Compare
normanmaurer
force-pushed
the
writable_method
branch
from
317af62 to
180ba49
Compare
normanmaurer
force-pushed
the
writability
branch
from
e3f118c to
0097732
Compare
normanmaurer
force-pushed
the
writable_method
branch
2 times, most recently
from
534f6a0 to
ec14bcb
Compare
Member
|
@normanmaurer Yup, let's make it a default method. Thanks a lot 🙇 |
normanmaurer
force-pushed
the
writable_method
branch
from
0872695 to
0bfd198
Compare
normanmaurer
force-pushed
the
writability
branch
2 times, most recently
from
baddf66 to
b8907a3
Compare
normanmaurer
force-pushed
the
writable_method
branch
from
0bfd198 to
9604222
Compare
normanmaurer
force-pushed
the
writable_method
branch
from
9604222 to
cd3d264
Compare
trustin
reviewed
Jul 1, 2022
Member
trustin
left a comment
trustin
left a comment
There was a problem hiding this comment.
Now that we added back isWritable(), we should revert most of the writableBytes() > 0 checks in this PR, because isWritable() has much lower cognitive load on the reader. I added them as suggestions, so you can easily revert them, although you'll have to optimize imports and make Checkstyle happy.
normanmaurer
changed the title
normanmaurer
force-pushed
the
writable_method
branch
from
449b664 to
7e15f47
Compare
normanmaurer
force-pushed
the
writable_method
branch
from
7e15f47 to
b4fb2f9
Compare
…ritable() default method Motivation: We should expose `Channel.writableBytes()` as a replacement for both, `Channel.bytesBeforeUnwritable()` and `Channel.isWritable()`. If the `Channel` is writable this method will return a positive number which can also give the user some idea how much data they can write without the risk of having the `Channel` become unwritable, if not writable it will return 0. Modifications: - Rename Channel.bytesBeforeUnwritabled() to writableBytes() and isWritable() default method - Adjust code Result: Less reduntant API and cleanup Co-authored-by: Chris Vest <christianvest_hansen@apple.com> Co-authored-by: Trustin Lee <t@motd.kr>
normanmaurer
force-pushed
the
writable_method
branch
from
b4fb2f9 to
6f5c5b6
Compare
Member
Author
|
Will merge this once green... |
violetagg
added a commit
to reactor/reactor-netty
that referenced
this pull request
Jul 4, 2022
…ritable() is removed netty/netty#12530
violetagg
added a commit
to reactor/reactor-netty
that referenced
this pull request
Jul 4, 2022
…ritable() is removed (#2351) - netty/netty#12530 - Temporary build on CI with PR in sock-proxy netty-contrib/socks-proxy#10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… isWritable()
Motivation:
We should expose
Channel.writableBytes()as a replacement for both,Channel.bytesBeforeUnwritable()andChannel.isWritable(). If theChannelis writable this method will return a positive number which can also give the user some idea how much data they can write without the risk of having theChannelbecome unwritable, if not writable it will return 0.Modifications:
Channel.bytesBeforeUnwritable()towritableBytes()and removeisWritable()Result:
Less reduntant API and cleanup