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

Allow app.ChainDecorators to accept nil #414

Closed
ethanfrey opened this issue Mar 16, 2019 · 1 comment · Fixed by #431
Closed

Allow app.ChainDecorators to accept nil #414

ethanfrey opened this issue Mar 16, 2019 · 1 comment · Fixed by #431
Assignees

Comments

@ethanfrey
Copy link
Contributor

ethanfrey commented Mar 16, 2019

Is your feature request related to a problem? Please describe.

If we want to optionally enable a decorator, we have to create a slice with all other decorators, and in an if statement, splice this in if needed.

Describe the solution you'd like

Simpler would be to use the if statement to return either the decorator or nil, and just pass that into ChainDecorators. Decorators.Handle() would just silently skip any nil Decorators as if they were not passed in. Makes a more convenient API with no run-time overhead.

@ruseinov
Copy link
Contributor

I'd go for returning (decorator, err) and have a predefined err like ErrDisabled or something similar to check on.

Could be somewhat cleaner than just nil check and more idiomatic.

@husio husio self-assigned this Mar 21, 2019
husio added a commit that referenced this issue Mar 21, 2019
For ease of configuration allow to pass a nil value decorator to the
decorator chain builder. Nil decorator must be ignored.

resolve #414
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 a pull request may close this issue.

3 participants