feat(store): add codegen for all types for tightcoder #396
Conversation
dk1a
force-pushed
the
dk1a/v2-storagecoder-codegen
branch
from
fbda72b
to
2320b6d
Compare
dk1a
changed the title
dk1a
force-pushed
the
dk1a/v2-storagecoder-codegen
branch
from
2320b6d
to
02a7a4b
Compare
There was a problem hiding this comment.
Great PR! Just very minor questions/nits.
Also, do you think this PR would be a good place to rename TightCoder into Serializer (as proposed by @holic in #387 (comment))?
| ************************************************************************/ | ||
| <% for (let i = 8; i <= 256; i += 8) { -%> | ||
|
|
||
| function decodeArray_uint<%= i %>(Slice self) internal pure returns (uint<%= i %>[] memory output) { |
There was a problem hiding this comment.
ooc why did you decide for this naming format (decodeArray_uint256) instead of something like decodeUint256Array? Also, since the library is called DecodeSlice, I would have expected the function to be called something like toUint256Array, but no strong opinion.
There was a problem hiding this comment.
In short I'm going for <method>_<T> as a way to resemble generics
why did you decide for this naming format (decodeArray_uint256) instead of something like decodeUint256Array?
The main reason is autogen. After _ comes the exact type name. I'd rather not do more manipulations than necessary since this is already quite complicated.
Also, since the library is called DecodeSlice, I would have expected the function to be called something like toUint256Array
Similar reason - I want the "method name" to be purely as a prefix, and "type name" purely as a postfix, both separated by _.
For encode/decode - I feel like while writing it I had a harder time remembering which methods did what with to/from. With decode I had a better analogy to abi.decode etc.
| @@ -0,0 +1,81 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity >=0.8.0; | |||
|
|
|||
There was a problem hiding this comment.
Should we put a comment on the top of the file saying something like "Autogenerated from DecodeSlice.ejs via yarn codegen, do not edit manually"?
There was a problem hiding this comment.
Added one, see if it's ok. I'm not sure we need the comment customized to filename, but mentioning which file shouldn't be edited seems important
(for schema autogen I just write /* Autogenerated file. Do not edit manually. */ but since here both files are together, it's not very clear)
Co-authored-by: alvarius <[email protected]>
I'd rather do such encompassing renamings closer to v2 completion when we have world and autogen merged, to avoid changing WIP branches often. Will give more time to think about the name too |
Depends on #395
Not sure what to do with autogenerated files, tentatively not gitignoring them for now