Conversation
|
Review requested:
|
nodejs-github-bot
added
c++
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jsnell/more-localvector
branch
from
cf7258c to
477c2dd
Compare
| strncmp(id, | ||
| "internal/bootstrap/", | ||
| strlen("internal/bootstrap/")) == 0) { | ||
| std::span<Local<String>> span(¶meters[0], arraysize(parameters)); |
There was a problem hiding this comment.
Not here, no. It's just a span over the array.
| &source, | ||
| params, | ||
| context_extensions, | ||
| parms, |
There was a problem hiding this comment.
Is this intentional? Why parms but not params?
There was a problem hiding this comment.
params and parms are different types.
| Environment* env_; | ||
| const std::vector<BaseObjectPtr<BaseObject>>& host_objects_; | ||
| const std::vector<Local<SharedArrayBuffer>>& shared_array_buffers_; | ||
| const std::span<Local<SharedArrayBuffer>>& shared_array_buffers_; |
There was a problem hiding this comment.
Some lines are changed to LocalVector some of them to std:span. It's hard to keep track the reason
There was a problem hiding this comment.
LocalVector is the storage. The std::span is just a view. In some cases the spans are views over a LocalVector, in other cases the spans are views over an array.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56445 +/- ##
==========================================
- Coverage 88.53% 88.03% -0.51%
==========================================
Files 657 657
Lines 190741 190752 +11
Branches 36607 36303 -304
==========================================
- Hits 168881 167920 -961
- Misses 15036 15958 +922
- Partials 6824 6874 +50
|
|
There's some complexity in this causing some CI failures. Going to close this and revisit later. |
3 participants
No description provided.