Use IPLD bindnode and reduce boilerplate code in ingest schema #268
Conversation
Codecov Report
@@ Coverage Diff @@
## main #268 +/- ##
===========================================
+ Coverage 29.34% 45.31% +15.96%
===========================================
Files 84 82 -2
Lines 11765 6117 -5648
===========================================
- Hits 3453 2772 -681
+ Misses 7817 2960 -4857
+ Partials 495 385 -110
|
|
Thanks for looking @gammazero. I have tagged Daniel as a review to check IPLD stuff. |
|
|
||
| // FromNode populates the fields of this advertisement from the given node. | ||
| func (a *Advertisement) FromNode(node ipld.Node) error { | ||
| if node.Prototype() != AdvertisementPrototype { |
There was a problem hiding this comment.
When do you expect this to kick in? I would document it.
If it's kicking in often, then I imagine something's not right - perhaps your link loading stuff is using basicnode.Prototype.Any when following links, and then you're doing the extra work of converting the untyped node to Advertisement here. At the very least this needs a TODO to avoid the duplicate work at some point in the future.
There was a problem hiding this comment.
The linksystem is passed around to other libraries like go-legs which might load things using Any and I don't have control over that unfortunately. All code in this repo, however uses appropriate prototype when loading.
There was a problem hiding this comment.
That's fine, I would just document it then. I think @willscott, you, and I could briefly chat about this at some point, so that we can properly plumb a NodePrototypeChooser through libraries like go-legs.
| } | ||
| EntryChunk struct { | ||
| Entries []multihash.Multihash | ||
| Next *ipld.Link |
There was a problem hiding this comment.
I know this pointer to interface feels weird, because an interface can already be nil, which should be enough for optional/nullable. See ipld/go-ipld-prime#378 FYI
There was a problem hiding this comment.
Great to see that issue captured 👍 In the meantime, I have to use pointer to interfaces right?
masih
force-pushed
the
masih/use-ipld-bindnode
branch
2 times, most recently
from
dd46a1c to
a326700
Compare
|
@gammazero that's all Daniel's review comments addressed, and branch rebased from This is ready for another look whenever you get the chance. |
Reflect changes in storetheindex that replaces IPLD codegen schema with bindnode structs. See: - ipni/storetheindex#268
Use IPLD bindnode to directly wrap and unwrap go structs in order to reduce boilerplate code when encoding or decoding ingest types. The codegen used before to generate IPLD schema types was also out of date. Using bindnode has the added advantage that there is no need to keep generated code up-to-date.
masih
force-pushed
the
masih/use-ipld-bindnode
branch
from
a326700 to
d35a321
Compare
Reflect changes in storetheindex that replaces IPLD codegen schema with bindnode structs. See: - ipni/storetheindex#268
Reflect changes in storetheindex that replaces IPLD codegen schema with bindnode structs. See: - ipni/storetheindex#268
Reflect changes in storetheindex that replaces IPLD codegen schema with bindnode structs. See: - ipni/storetheindex#268
Reflect changes in storetheindex that replaces IPLD codegen schema with bindnode structs. See: - ipni/storetheindex#268
Use IPLD bindnode to directly wrap and unwrap go structs in order to
reduce boilerplate code when encoding or decoding ingest types.
The codegen used before to generate IPLD schema types was also out of
date. Using bindnode has the added advantage that there is no need to
keep generated code up-to-date.