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

Multiple error handler labels #454

Closed
wants to merge 7 commits into from
Closed

Conversation

johakoch
Copy link
Collaborator

config.ErrorHandler with only one Kind; configload.kindContent with only one kind; avoid double iteration


Reviewer checklist
  • Read PR description: a summary about the changes is required
  • Changelog updated
  • Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage
  • Pulled branch, manually tested
  • Verified requirements are met
  • Reviewed the code
  • Reviewed the related tests

@johakoch johakoch marked this pull request as ready for review March 16, 2022 21:15
@johakoch johakoch marked this pull request as draft March 16, 2022 21:15
@johakoch johakoch force-pushed the multiple-error_handler-labels branch from 47c54cd to 2ae85de Compare March 23, 2022 14:07
@johakoch johakoch force-pushed the multiple-error_handler-labels branch from 2ae85de to 548e2ee Compare March 24, 2022 13:35
@johakoch
Copy link
Collaborator Author

errorHandlerContent's map key (the kind) is here only used for the uniqueness check. To get the actual kind kindContent.kind is used.

Another approach:

type errorHandlerContent map[string]hcl.Body
  • use the map key as actual kind: pass both kind string and body hcl.Body instead of content kindContent to newErrorHandlerConfig()
  • use them instead of content.kind and content.body
  • the kindContent type is no longer necessary

@malud
Copy link
Collaborator

malud commented Apr 4, 2022

closed in favor of #462

@malud malud closed this Apr 4, 2022
@malud malud deleted the multiple-error_handler-labels branch April 4, 2022 10:02
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