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

Struct Tag Support #2

Closed
jmank88 opened this issue Apr 17, 2017 · 7 comments
Closed

Struct Tag Support #2

jmank88 opened this issue Apr 17, 2017 · 7 comments

Comments

@jmank88
Copy link
Owner

jmank88 commented Apr 17, 2017

Support basic json struct tags, and any relevant options (e.g. omitempty).

Consider an option for alternate type encoding (e.g. tag a byte with 'C', rather than use a Char type).

Keep an eye on the benchmarks - consider caching struct type info on the first run (fields, tags, counts, etc).

@peergynt
Copy link

peergynt commented Jan 4, 2018

Hi Jordan, are you still interested in implementing some support for field tags? I cannot use auto decoding when all my keys are lower case strings. I would be willing to take on this issue.

@jmank88
Copy link
Owner Author

jmank88 commented Jan 8, 2018

Yes, I'm still interested in this. I didn't have firm plans for this when I wrote it up, so it may need some more design up front.

One thing I was wondering was if it make sense (and is it possible) to leverage the same json tag? Or would it be better to have a separate ubjson tag? It would be convenient to be able to use previously json tagged structs without any extra work, but I worry that options could become confusing or even incompatible. At the least, I think that we'd need to be able to override them with something specific to ubjson. Perhaps ubjson tags could override/augment json tags when both are on a single field. This would allow a simpler incremental first step of just leveraging the json tag names, which is most important, and the rest can be sorted out later.

Also related is #6. If there is a significant speed boost, it'd be neat to generate Value code from the structs, including tags. Mostly orthogonal to this, but some of the code would be leveraged from that pathway, so just something to keep in mind.

You are welcome to work on this issue. Feel free to put a WIP pull request, or let me know if you get stuck or need any help.

@jmank88
Copy link
Owner Author

jmank88 commented Jan 12, 2018

@peergynt, have you started on this issue? If not, I prototyped something that I can put up soon.

@peergynt
Copy link

I did not really start anything yet. This is kind of a side-project of a side-project...
However I had some initial thoughts.

I was thinking that the json tag belonged to the 'encoding/json' package so it might make sense to use a new tag like ubjson. It is not uncommon for fields to have multiple tags: json, xml, form...
However I agree that since UBJSON is very much related to JSON, you can make a case for using the json tag.

As far as the design, I was thinking about imitating the 'encoding/json' implementation (i.e. tag caching...).

In any case, let me know what I can help with.

@jmank88
Copy link
Owner Author

jmank88 commented Jan 15, 2018

@peergynt I put up #8, if you'd like to take a look or test the branch.

After thinking more about it, the duplication from having a separate 'ubjson' tag isn't such a big deal, plus recognizing 'json' first could potentially force people to add an additional 'ubjson' tag just to get back to the original field name.

@peergynt
Copy link

@jmank88 I tested the struct_tag branch and the ubjson tag seems to work well!

@jmank88
Copy link
Owner Author

jmank88 commented Jan 19, 2018

Closing this since basic support has been added. Can revisit if more options/extensions are needed later on.

@jmank88 jmank88 closed this as completed Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants