Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

feat: Implement Diagnostics.BySeverity filtering #288

Merged
merged 1 commit into from
May 25, 2022
Merged

Conversation

disq
Copy link
Member

@disq disq commented May 25, 2022

🎉 Thank you for making CloudQuery awesome by submitting a PR 🎉

Summary

Adds helper method to filter diag.Diagnostics by multiple severity levels.


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run --new-from-rev main 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@disq disq requested a review from roneli May 25, 2022 08:37
@disq disq requested a review from a team as a code owner May 25, 2022 08:37
@github-actions github-actions bot added the feat label May 25, 2022
Copy link
Contributor

@roneli roneli left a comment

Choose a reason for hiding this comment

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

I liked 🚀 , LGTM

@disq disq merged commit 75213de into main May 25, 2022
@disq disq deleted the feat/diag-byseverity branch May 25, 2022 08:39
@@ -152,6 +152,24 @@ func (diags Diagnostics) CountBySeverity(sev Severity, includeSquashed bool) uin
return count
}

// BySeverity returns a subset of diagnostics matching the given severity.
func (diags Diagnostics) BySeverity(sevs ...Severity) Diagnostics {
Copy link
Member

@erezrokah erezrokah May 25, 2022

Choose a reason for hiding this comment

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

Nice 🚀

I actually started something like:

func (diags Diagnostics) Filter(filterFunc func(d Diagnostic) bool) Diagnostics {
	res := make(Diagnostics, 0, len(diags))
	for _, d := range diags {
		if filterFunc(d) {
			res = append(res, d)
		}
	}
	return res
}

But that could be too generic

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about that, maybe generics can solve it later :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants