Skip to content

daemon/logger: un-export RingLogger#48893

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:logger_cleans
Jan 15, 2025
Merged

daemon/logger: un-export RingLogger#48893
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:logger_cleans

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Nov 18, 2024

It doesn't look like this type was intended for external use; constructing a RingLogger can be done through the NewRingLogger() constructor, which returns a Logger interface (implemented by RingLogger).

- Description for the changelog

daemon: un-export RingLogger as it is only used internally. Use NewRingLogger() instead.

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the 28.0.0 milestone Nov 18, 2024
@thaJeztah thaJeztah marked this pull request as ready for review November 18, 2024 14:42
It doesn't look like this type was intended for external use; constructing
a RingLogger can be done through the `NewRingLogger()` constructor, which
returns a `Logger` interface (implemented by `RingLogger`).

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Comment thread daemon/logger/ring.go
Comment on lines 25 to 26
var (
_ SizedLogger = (*RingLogger)(nil)
_ SizedLogger = (*ringLogger)(nil)
_ LogReader = (*ringWithReader)(nil)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually wondering if (but for a follow-up) NewRingLogger should explicitly return a SizedLogger, because that's what's implemented by both;

// NewRingLogger creates a new Logger that is implemented as a RingBuffer wrapping
// the passed in logger.
func NewRingLogger(driver Logger, logInfo Info, maxSize int64) Logger {

And perhaps assert that here as well for ringWithReader;

	_ SizedLogger = (*ringWithReader)(nil)

@thaJeztah
Copy link
Copy Markdown
Member Author

Let me bring this one in; I'll have a look at those other changes in a follow up if I find some time 👍

@thaJeztah thaJeztah merged commit 079b6e6 into moby:master Jan 15, 2025
@thaJeztah thaJeztah deleted the logger_cleans branch January 15, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants