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

Reporter API v2 #2191

Closed
geofjamg opened this issue Jul 3, 2022 · 0 comments · Fixed by #2201
Closed

Reporter API v2 #2191

geofjamg opened this issue Jul 3, 2022 · 0 comments · Fixed by #2201
Labels

Comments

@geofjamg
Copy link
Member

geofjamg commented Jul 3, 2022

  • Do you want to request a feature or report a bug?
    Feature

  • What is the current behavior?
    Some part of the API could be improved/refactored to be more clear.

  • What is the expected behavior?

  1. report and reporter are too close (I mean in term of naming) and very confusing. When we talk about logging we have a logger and with a logger with can log and we log a message. In report API we should follow the same and we should rename Report to Message
reporter.report(Message.builder()......build())
  1. when creating a child reporter we set a taskId. This is confusing too because , nowhere we have task. we assign a taskId to a reporter. I think this was just to avoid naming it reporterId which was too close to reportId. But if Report is renamed to Message using a messageId, there is no problem to have a reporterId.
  2. [LESS IMPORTANT] Maybe also we should reuse the Message class to create the child reporter to avoid reinventing similar concept for Reporterwith kind of "tilteMessage" or "headerMessage" like:
var child = reporter.createChild(Message.builder()....build());
  • What is the motivation / use case for changing the behavior?

  • Please tell us about your environment:

    • PowSyBl Version: ...
    • OS Version: ...
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, spectrum, etc)

(if a question doesn't apply, you can delete it)

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

Successfully merging a pull request may close this issue.

1 participant