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

Fix: Set the scrape manager #2

Merged
merged 1 commit into from
Mar 30, 2020
Merged

Conversation

gotjosh
Copy link

@gotjosh gotjosh commented Mar 30, 2020

I completely missed this in my original PR. I believe I lost it on my last rebase from master to FF my original branch.

I'm a bit surprised that gofmt didn't yell at something like this. As for a test, I couldn't come up with a good test to ensure this does not happen again other than setting it and then ensure it is there - but that seems unnecessary? Opinions are welcome.

@gotjosh gotjosh requested review from cstyan and rfratto March 30, 2020 17:04
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Unfortunately making sure something is initialized is generally out of scope of Go tooling, even linters. While languages like Rust force everything to have a default value, Go's convention of the zero value being useful leads to situations like this that can't be covered by lint rules and are easy to miss.

LGTM!

@gotjosh gotjosh merged commit fc3d977 into shared-interner Mar 30, 2020
@gotjosh gotjosh deleted the set-scrape-manager branch March 30, 2020 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants