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

Handle JSON func values #835

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zachbadgett
Copy link

Leave func handling up to the formatter.

Leave func handling up to the formatter.
Errors can be handled field by field.
@zachbadgett zachbadgett changed the title Move func reflect check to json formatter Handle JSON func values Oct 4, 2018
@zachbadgett
Copy link
Author

In regards to #832 and #642; no need to clutter our entry with more fields. Let the formatter decide how the field types are handled and replace invalid values with a message.

@dgsb
Copy link
Collaborator

dgsb commented Oct 4, 2018

Hello @zachbadgett, thanks for your contribution, but I'm not sure if it's better to handle that at the formatter level rather than the Entry level.
We have another set of issues/pr (#365,#151,#207) related to fields for which it makes more sense to implement them at Entry level. I'd rather rationalize all this future work at entry level

@zachbadgett
Copy link
Author

IMO, which types to support would best to be left to the formatter. If a user wants to implement a custom formatter and wants to support grabbing details from a custom function in their application then they can.

Either way, I believe you should either move all normalization to the entry (which includes the error to string) or keep it all on the formatter.

@dgsb
Copy link
Collaborator

dgsb commented Oct 4, 2018

IMO It's kind of different, an error can be stringified. The fact that the json formatter can't is an implementation detail of the json formatter.
While I'm not sure it does not make sense to stringify a function even if the TextFormatter can do it.

@zachbadgett
Copy link
Author

Kind of a silly example if someone really wanted to get information on a func: https://play.golang.org/p/zJiQT7AF-ni
I do understand your point about if a func can really be "stringified", the thing I was trying to get at is an entry should be type agnostic, let the formatters worry about which type to support.

@stale stale bot added the stale label Feb 26, 2020
@markphelps markphelps removed the stale label Feb 26, 2020
Repository owner deleted a comment from stale bot Feb 26, 2020
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