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

Review naming of FrameHandler.isDemanding() in Jetty 12 #8991

Closed
lachlan-roberts opened this issue Dec 1, 2022 · 2 comments
Closed

Review naming of FrameHandler.isDemanding() in Jetty 12 #8991

lachlan-roberts opened this issue Dec 1, 2022 · 2 comments
Labels
Enhancement

Comments

@lachlan-roberts
Copy link
Contributor

Currently FrameHandler has a method called isDemanding() which specifies whether a FrameHandler will have to manage its own flow control by using the CoreSession.demand() method.

demanding == false means that demand is combined with the callback.
demanding == true means that frame handler must use demand() method.

Simone says this is confusing and this should be renamed in Jetty 12.
see #8487 (comment)
@gregw
Copy link
Contributor

gregw commented Dec 1, 2022

Kind of agree the name (out at least the sense of the boolean) is messed up (yes it was my name).

Something like isAutoDemand would be clearer.

@joakime joakime mentioned this issue Dec 19, 2022
Closed
lachlan-roberts added a commit that referenced this issue Feb 8, 2023
@lachlan-roberts
Issue #8991 - rename websocket isDemanding() method to isAutoDemanding()
d2dcbeb 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts mentioned this issue Feb 8, 2023
Merged
lachlan-roberts added a commit that referenced this issue Feb 9, 2023
@lachlan-roberts
Merge pull request #9328 from eclipse/jetty-12.0.x-websocket-autodema…
55357f4 
…nding

Issue #8991 - rename websocket isDemanding() method to isAutoDemanding()
gregpoulos pushed a commit to gregpoulos/jetty.project that referenced this issue Feb 9, 2023
Merge remote-tracking branch 'upstream/jetty-12.0.x' into jetty-12.0.…
2402b6a 
…x-documentation-operations-logging

* upstream/jetty-12.0.x: (35 commits)
  Fixes jetty#9326 - Rename DecryptedEndPoint to SslEndPoint.
  Jetty 10 Upgrade to Hazelcast 5 and totally disable auto join multicast etc.. (fix build on CI) (jetty#9331)
  jetty#9328 - changes from review
  jetty#9287 - catch error in ee9 maxRequestSize MultiPart test
  Jetty 12.0.x 9301 fix ee10 jstl jpms (jetty#9321)
  Issue jetty#9301 Fix dependencies for ee10-glassfish-jstl module (jetty#9303)
  Jetty 12 Hazelcast 5.x and disable auto detection/multicast" (jetty#9332)
  jetty#9287 - fix further test failures
  Fixed imports.
  Issue jetty#7650 - Fix race condition when stopping QueuedThreadPool (jetty#9325)
  jetty#9287 - remove unpaired release of Content.Chunk
  Issue jetty#8991 - rename websocket isDemanding() method to isAutoDemanding()
  Issue jetty#9287 - fix failing tests
  changes f rom review
  add todo to revert to normal pool after fix for jetty#9311
  Issue jetty#9309 - Introducing test for requestlog format with spaces
  use non-pooling RetainableByteBufferPool to work around performance bug
  consumeAvailable should use number of reads instead of bytes
  fix for retainable merge
  changes from review
  ...
@sbordet
Copy link
Contributor

sbordet commented Feb 13, 2023

Fixed by #9328.

@sbordet sbordet closed this as completed Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gregw @sbordet @lachlan-roberts