Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

UNDERTOW-1794 DefaultAccessLogReceiver violates Closeable contract #1593

Open
wants to merge 2 commits into
base: 2.2.x
Choose a base branch
from
Open

UNDERTOW-1794 DefaultAccessLogReceiver violates Closeable contract #1593

wants to merge 2 commits into from

Conversation

baranowb
Copy link
Contributor

@baranowb baranowb force-pushed the UNDERTOW-1794_2.2.x branch from db0351a to ace463d Compare July 10, 2024 07:03
@ropalka ropalka requested review from ropalka and fl4via August 6, 2024 17:39
@baranowb baranowb added enhancement Enhances existing behaviour or code new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) waiting peer review PRs that edit core classes might require an extra review labels Sep 4, 2024
@fl4via fl4via added the under verification Currently being verified (running tests, reviewing) before posting a review to contributor label Oct 21, 2024
jasondlee
jasondlee reviewed Nov 26, 2024
View reviewed changes
core/src/main/java/io/undertow/server/handlers/accesslog/DefaultAccessLogReceiver.java
Comment on lines +229 to +230
if(bw != null)
bw.flush();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this block have curlies? Not sure what any coding styles for Undertow might say, but I find they're safer to have.

jasondlee
jasondlee approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@jasondlee jasondlee left a comment

Choose a reason for hiding this comment

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

Other than a very minor style comment/question, this looks good to me.

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jasondlee jasondlee jasondlee approved these changes

@ropalka ropalka Awaiting requested review from ropalka

@fl4via fl4via Awaiting requested review from fl4via

Assignees
No one assigned
Labels
enhancement Enhances existing behaviour or code new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting peer review PRs that edit core classes might require an extra review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@baranowb @jasondlee @fl4via