Open Sytten opened 1 month ago
Attention: Patch coverage is 81.02981%
with 70 lines
in your changes are missing coverage. Please review.
Project coverage is 68.75%. Comparing base (
304db5d
) to head (8b80197
).
Files | Patch % | Lines |
---|---|---|
core/src/value/iterator/iterable.rs | 70.33% | 35 Missing :warning: |
core/src/value/iterator.rs | 88.69% | 26 Missing :warning: |
core/src/value/iterator/into_iter.rs | 57.14% | 9 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Gentle ping @DelSkayn, whenever you have time I just want to avoid it getting stale :)
This is my first attempt at https://github.com/DelSkayn/rquickjs/issues/318
Couple of points before reviewing:
ArrayBuffer
sinceIterable
andIterator
are not JS types and I didn't feel comfortable adding them to theType
enumIntoJsIter
since it's not really anInto
trait like we generally see in rust, but it's similar in spirit toIntoJsFunc
IntoJsIter
are a bit strange with theposition
. I use theposition
in my util crate https://github.com/DelSkayn/rquickjs/pull/319/files#diff-653c6889323b6067b2be818f8ff00f785927ae3beaaeaba4a034feb5ea154d4eR133 since I do some magic with theThis
to access the parent class to avoid copying the values and my function is stateless so I need some indication on where we are. But we might want to remove it from the trait and move it toIterFn
andIterFnMut