sheriff
sheriff copied to clipboard
liip
nil handling
pinging @lombare @simaotwx @TorbenCK @mandeepji
We have several conflicting opinions about how to handle nil and/or empty maps/slices: see the following PRs:
- #37
- #38
- #39
- #40
I accidentally merged again something which was previously called a regression just before (sorry about that); will revert that change.
What is your opinion on how this should be handled? Would we need additional flags to capture this? Just a new version (github.com/liip/sheriff/v1) to make sure there aren't regressions in existing codebases?
Please let me know your thoughts. Thanks.
Yes, the change might be small, but might not be minor. It would lead to a regression, if a client expects that arrays are never null.
It is the same issue that we have, but reversed. We want to use sheriff but want to ensure that clients receive the same response as with json.Marshal.
I think a new version would be good
Another solution would be to make sheriff configurable. By default it will have the old behaviour, but the json.Marshal conform behaviour can be activated over configuration.
This should also cover the other issues you mentioned. Maybe it would also make sense to add a test case, that runs json.Marshal vs. sheriff.Marshal, to really exclude further differences.
What do you think?
I agree with TorbenCK, the V2 sounds like a good idea since it won't break existing codebases
To feed the debate, I think that the good behavior is to have the same as json.Marshal (since his sole goal is to comply with the RFC) To me the role of the Marshaler is to make a representation of a data, so if given null, then outs null and not {} or [].
Just wanted to follow-up as we are interested in a near-term solution for the problem. Can we release https://github.com/liip/sheriff/pull/43 soon?
@mweibel May I know what is next step? Are we waiting for any changes/feedback or planning to merge #43 ?
I read all the comments and will work on it as soon as I find the time to do so. I hope I find some time this week but can't guarantee it. In the meantime, you could work on a PR if you wish to do so (and use it using the replace directive of Go modules).
just wanted to follow-up. I am not sure what still needs to be done but hope someone will find time soon, so we can utilize this lib in our solution :)
I released github.com/liip/sheriff/v2 in version v2.0.0-beta.1 just now. Sorry about the delay!
Had to bump it to v2 directly (instead of making a v1 first) because v0 and v1 are somehow synonymous. Not sure why but go modules are sometimes still a mystery to me ;)
Let me know how that goes, I intentionally released it as beta so we can still make changes if necessary.
ah yeah FTR, I didn't opt for options to control this behaviour. I believe this is too complicated. We should just strive to do whatever json.Marshal does IMO.