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: Strict thinks Quiet Sev is an error #157

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

xorima
Copy link
Contributor

@xorima xorima commented Nov 4, 2023

When run with --strict today an error is reported even if it is a quiet issue, this will move quiet under warning so that will not trigger anymore.

Fixed #156

Bug:

When you run the linter with --strict it reports failures even though from a ui perspective nothing is visible as a failure.

Analysis:

All of the Severity levels are iota based with Success being 0 and importantly Quiet being 4

https://github.com/grafana/dashboard-linter/blob/main/lint/lint.go#L11

The Results object is processed for MaximumSeverity and looks for the highest number based on iota: https://github.com/grafana/dashboard-linter/blob/main/lint/results.go#L160 which makes Quiet higher than the default Success so it is returned with the response of 4

Finally when strict is enabled, it will report errors for any level above Warning, https://github.com/grafana/dashboard-linter/blob/main/main.go#L91 which Quiet is, so errors are reported but they are not visible at all anywhere

Possible fixes:

  1. Update the MaximumSeverity to ignore Quiet - This may give people surprises later
  2. Move Quiet to iota 1 so it will be: Success, Quiet,
  3. If in strict mode not return if the result = Quiet. - This has a flaw that today Quiet is considered a higher level than error so errors could be hidden from strict response

Chosen fix: 2 - Change the iota and validate other tests don't fail

When run with `--strict` today an error is reported even if it is a
quiet issue, this will move quiet under warning so that will not trigger
anymore.

Fixed grafana#156

Signed-off-by: Jason Field <[email protected]>
Copy link
Collaborator

@rgeyer rgeyer left a comment

Choose a reason for hiding this comment

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

Ahh, good catch. That was a subtle oversight!

Thank you!

@rgeyer rgeyer merged commit c458893 into grafana:main Nov 14, 2023
4 checks passed
@xorima xorima deleted the fix/strict-errors branch November 15, 2023 01:24
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.

Bug - Strict reports errors found when quiet is in use
2 participants