Skip to content

sqlite: refactor error helpers and user function pointers - #62794

Open
thisalihassan wants to merge 1 commit into
nodejs:mainfrom
thisalihassan:sqlite-refactor-error-and-udf-ptrs
Open

sqlite: refactor error helpers and user function pointers#62794
thisalihassan wants to merge 1 commit into
nodejs:mainfrom
thisalihassan:sqlite-refactor-error-and-udf-ptrs

Conversation

@thisalihassan

Copy link
Copy Markdown
Contributor

Two small independent refactors.

  1. Consolidate CreateSQLiteError overloads
  2. BaseObjectWeakPtr for user functions (style consistency)

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Apr 17, 2026
@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (54a5095) to head (fe5a33c).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 88.88% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #62794   +/-   ##
=======================================
  Coverage   90.14%   90.14%           
=======================================
  Files         744      744           
  Lines      242518   242500   -18     
  Branches    45685    45683    -2     
=======================================
- Hits       218611   218606    -5     
+ Misses      15396    15390    -6     
+ Partials     8511     8504    -7     
Files with missing lines Coverage Δ
src/node_sqlite.h 80.64% <ø> (ø)
src/node_sqlite.cc 80.72% <88.88%> (+0.05%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thisalihassan

Copy link
Copy Markdown
Contributor Author

@nodejs/sqlite @geeksilva97 can I get a review please?

@geeksilva97

Copy link
Copy Markdown
Contributor

Sure. Will take a look.

@geeksilva97

Copy link
Copy Markdown
Contributor

cc @nodejs/cpp-reviewers

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73204/

@geeksilva97
geeksilva97 requested a review from addaleax May 7, 2026 18:29
@trivikr

trivikr commented Jul 25, 2026

Copy link
Copy Markdown
Member

Is this okay to land @geeksilva97, or do we need an additional review from CPP team?

@geeksilva97

geeksilva97 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Is this okay to land @geeksilva97, or do we need an additional review from CPP team?

PR LGTM. We can get it landed, I was just looking for another eye before moving further.

We need a new CI run since it's been a while

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Querying data for job/node-test-pull-request/73204/
[SyntaxError: Unexpected token '<', ..."    
  https://github.com/nodejs/node/actions/runs/30204902769

@thisalihassan

Copy link
Copy Markdown
Contributor Author

@geeksilva97 @trivikr do I need to rebase or something?

@geeksilva97

Copy link
Copy Markdown
Contributor

@geeksilva97 @trivikr do I need to rebase or something?

yes, please

Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
@thisalihassan
thisalihassan force-pushed the sqlite-refactor-error-and-udf-ptrs branch from 7d3237b to fe5a33c Compare July 27, 2026 08:44
@geeksilva97 geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 27, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Querying data for job/node-test-pull-request/73204/
[SyntaxError: Unexpected token '<', ..."    
  https://github.com/nodejs/node/actions/runs/30286474954

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants