-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
src,sqlite: replace tolocalchecked with tolocal #60028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60028 +/- ##
==========================================
- Coverage 88.47% 88.47% -0.01%
==========================================
Files 703 703
Lines 207546 207551 +5
Branches 40006 40001 -5
==========================================
- Hits 183631 183622 -9
- Misses 15907 15912 +5
- Partials 8008 8017 +9
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this was still one of the open issues for the session extension integration.
I think this implementation makes it possible that a changeset is being applied partially. Not a big deal, the user could just wrap it in a transaction. Just something to be aware of.
I think we should do a rollback if any exception is being thrown from either the filter callback or the conflict handler before stabilizing the SQLite API. But for now, this is an improvement.
Failed to start CI⚠ Commits were pushed since the last approving review: ⚠ - src,sqlite: replace tolocalchecked with tolocal ⚠ - src: implement addaleax suggestions ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/18080613898 |
PR-URL: #60028 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 4136934 |
PR-URL: #60028 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This PR replaces
ToLocalChecked
withToLocal
in multiple places so we prevent unexpected crashes.