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

[chore] prometheusremotewrite: simplify wal initialization #30631

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

bogdandrutu
Copy link
Member

In the only production code that calls the newWal we first verify that the config is not nil then call into the newWal that returns error if config is nil which means that code never runs in production, so better to simplify the newWal and return nil if error is nil.

Alternative we can just remove the nil check in newWal and rely on the caller to check for the nil config.

@atoulme
Copy link
Contributor

atoulme commented Jan 17, 2024

LGTM, lint fails because error is now always nil, can be dropped from return arguments.

@bogdandrutu
Copy link
Member Author

@rapphil @Aneurysm9 can you review?

@bogdandrutu
Copy link
Member Author

Waited 1 week for the codeowners review, and this is a simple PR so I use my maintainer "hat" and decide to merge this.

@bogdandrutu bogdandrutu merged commit f07bcda into open-telemetry:main Jan 23, 2024
85 checks passed
@bogdandrutu bogdandrutu deleted the newwal branch January 23, 2024 17:39
@github-actions github-actions bot added this to the next release milestone Jan 23, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
…metry#30631)

In the only production code that calls the `newWal` we first verify that
the config is not nil then call into the `newWal` that returns error if
config is nil which means that code never runs in production, so better
to simplify the newWal and return nil if error is nil.

Alternative we can just remove the nil check in `newWal` and rely on the
caller to check for the nil config.

Signed-off-by: Bogdan Drutu <[email protected]>
anthoai97 pushed a commit to anthoai97/opentelemetry-collector-contrib that referenced this pull request Feb 12, 2024
…metry#30631)

In the only production code that calls the `newWal` we first verify that
the config is not nil then call into the `newWal` that returns error if
config is nil which means that code never runs in production, so better
to simplify the newWal and return nil if error is nil.

Alternative we can just remove the nil check in `newWal` and rely on the
caller to check for the nil config.

Signed-off-by: Bogdan Drutu <[email protected]>
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.

None yet

2 participants