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

refactor(api): Evaluators #431

Merged
merged 3 commits into from
Nov 27, 2020
Merged

refactor(api): Evaluators #431

merged 3 commits into from
Nov 27, 2020

Conversation

Duologic
Copy link
Member

With inline environments, I introduced quite a bit of complexity to the code base. This is a first attempt at trying to
reduce the complexity.

I've observed this pattern in https://github.com/grafana/cortex-tools/blob/master/pkg/rules/parser.go#L27-L37

@malcolmholmes
Copy link
Collaborator

Named funcs, much clearer.

Why though use a string and a switch? You could just pass in the function itself. The callers wouldn't notice the change from string to func, no?

pkg/tanka/parse.go Outdated Show resolved Hide resolved
pkg/tanka/parse.go Outdated Show resolved Hide resolved
Copy link
Member

@sh0rez sh0rez left a comment

Choose a reason for hiding this comment

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

Very nice!

pkg/tanka/parse.go Outdated Show resolved Hide resolved
@sh0rez
Copy link
Member

sh0rez commented Nov 27, 2020

@Duologic it is not documented at the moment, but I usually refer to pkg/tanka as api in conventional commits scope, so this PR would be refactor(api): redurce ..., and I'd even suggest refactor(api): Evaluators

@Duologic Duologic changed the title refactor: reduce complexity in parse.go refactor(api): Evaluators Nov 27, 2020
@Duologic Duologic merged commit 94e6f51 into master Nov 27, 2020
@Duologic Duologic deleted the duologic/less_complex_parse branch November 27, 2020 12:23
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

3 participants