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

Do not lint duplicate rules when checking Prometheus config. #10809

Closed
wants to merge 1 commit into from

Conversation

roidelapluie
Copy link
Member

Linting rules by default is a breaking change that broke our users' CI
pipeline with false positive. Let's exclude this by default.

Signed-off-by: Julien Pivotto roidelapluie@o11y.eu

Linting rules by default is a breaking change that broke our users' CI
pipeline with false positive. Let's exclude this by default.

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
@roidelapluie roidelapluie requested a review from dgl as a code owner June 3, 2022 09:15
@roidelapluie
Copy link
Member Author

cc @metalmatze can we cherry pick this in 2.36 and issue a new 2.36.1 with the go release update + this ?

@dgl
Copy link
Member

dgl commented Jun 3, 2022

Have you got details on what the breakage is? We did previously lint duplicate rules, but potentially the exit code changed... I'm worried if we do this we might break someone else in a different way.

@roidelapluie
Copy link
Member Author

yes, the exit code change is the problem here. we return non zero.

@roidelapluie
Copy link
Member Author

Have you got details on what the breakage is? We did previously lint duplicate rules, but potentially the exit code changed... I'm worried if we do this we might break someone else in a different way.

What would you propose?

@dgl
Copy link
Member

dgl commented Jun 3, 2022

What would you propose?

Maybe a --lint-fatal boolean flag to make lint errors exit with a fatal exit code, which can default to off. This should be consistent between check rules and check config (this PR is missing the check rules one at the moment).

Also, it looks like this was introduced in 2.35, not just 2.36.

@dgl
Copy link
Member

dgl commented Jun 3, 2022

Implemented my suggestion in #10815

@dgl dgl closed this Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants