Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
makew0rld
left a comment
There was a problem hiding this comment.
I'll wait until this PR is ready to do an in depth review, but if it's possible I think it would be a lot more idiomatic to have Document and Resource made up of public fields (with cbor struct tags). That way the developer can read and write data as they need easily. Functions like GetAttribute and SetAttribute could be removed in lieu of builtin map functionality. Functions that do something more advanced could remain.
Ideally one struct would be able represent both single file and multi file documents under this scheme, although I'm not sure how that would look. Maybe just some unused fields in either case?
a1c6f71 to
53995d5
Compare
|
Ahead of my review: the micro-benchmarks seem to indicate some large performance regressions when marshalling structs, please investigate that if you can. I can also look into it later on. |
makew0rld
left a comment
There was a problem hiding this comment.
See my comments below. Note CI tests are failing.
745a4ad to
1753899
Compare
1753899 to
198febb
Compare
makew0rld
left a comment
There was a problem hiding this comment.
Looks good, although let's merge the other PR first since this one relies on that.
Benchmark ResultsPerformance Regression Analysis |
No description provided.