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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LogStorage utility methods that expose the functionality of BufferedBuildListener, DelayBufferedOutputStream, and GCFlushedOutputStream #297

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

dwnusbaum
Copy link
Member

LogStorage implementations that are similar to FileLogStorage need to be able to use the same logic as BufferedBuildListener, DelayBufferedOutputStream, and GCFlushedOutputStream for both performance and correctness reasons, so we should expose these classes' functionality.

I think we can just add two methods to wrap a given OutputStream as needed without having to fully expose the classes. I am not sure what name would be best, feel free to recommend whatever you like.

Testing done

Submitter checklist

Preview Give feedback

…redBuildListener, DelayBufferedOutputStream, and GCFlushedOutputStream
@dwnusbaum dwnusbaum requested a review from a team July 20, 2023 14:04
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Allows fixes made for #294 to be used by custom storage systems.

@jglick jglick changed the title Add LogStorage utility methods that expose the functionality of BufferedBuildListener, DelayBufferedOutputStream, and GCFlushedOutputStream Add LogStorage utility methods that expose the functionality of BufferedBuildListener, DelayBufferedOutputStream, and GCFlushedOutputStream Jul 20, 2023
@jglick
Copy link
Member

jglick commented Jul 20, 2023

I am not sure what name would be best

The method names chosen seem reasonable to me.

@jglick jglick merged commit 4edc8b4 into jenkinsci:master Jul 20, 2023
@dwnusbaum dwnusbaum deleted the expose-utility-classes branch July 20, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants