Indexer #49
Indexer #49
Conversation
| code: &str, | ||
| raw: &[u8], | ||
| index: u32, | ||
| mut ondex: Option<u32>, |
There was a problem hiding this comment.
Instead of making optional params just yet, I have usually opted for explicit functions that either have the param or don't. In the (possibly near) future, we can, if it is necessary during higher level composition, add wrappers that check for the existence of the optional and delegate to the appropriate function under the hood (think new() without qualifiers, and optional params just like the__init__() methods in Python). This gives us more flexibility in composition within CESR, and will likely allow you to clean up some if nesting in this process. Something to consider.
m00sey
force-pushed
the
indexer
branch
2 times, most recently
from
112ce40 to
d596f39
Compare
Codecov Report
@@ Coverage Diff @@
## main #49 +/- ##
==========================================
- Coverage 97.83% 92.36% -5.47%
==========================================
Files 9 11 +2
Lines 968 1283 +315
==========================================
+ Hits 947 1185 +238
- Misses 21 98 +77
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
m00sey
force-pushed
the
indexer
branch
3 times, most recently
from
2bbb7bb to
d250457
Compare
There was a problem hiding this comment.
Looks really good, only a few minor comments. I don't think any are critical and since we still need to finish this off in a following PR, I'll approve here and you can decide if you want to merge as is and make any tweaks later, or touch up. I can re-review if you need.
No description provided.