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

package.parser: make Or linter work with arbitrary number of linters #324

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Mar 7, 2022

Description of your changes

We need this in provider linter where we need to allow three distinct types, CustomResourceDefinition, ValidatingWebhookConfiguration and MutatingWebhookConfiguration.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Unit tests. No breaking change in the interface only the format of the printed error is different now.

…instead of only two

Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf muvaf requested review from negz and hasheddan March 7, 2022 14:03
return errors.New(errNilLinterFn)
}
err := l(o)
if err == nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a behavioral difference that if any of the linters pass, we don't execute the next ones at all, which is I believe more in-line with how OR behavior is implemented in general.

}
return errors.Errorf(errOrFmt, aErr, bErr)
return errors.Errorf(errOrFmt, strings.TrimSuffix(errSet, ", "))
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Could you just build a slice of strings then join them, to avoid needing to trim?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I knew there was a better way to do this, thanks!

… the flow simpler

Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf muvaf merged commit b35cdab into crossplane:master Mar 9, 2022
@muvaf muvaf deleted the parser-or branch March 9, 2022 08:26
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.

2 participants