Skip to content

sqlite: add location method - #57860

Merged
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
geeksilva97:sqlite-add-location-function
Apr 21, 2025
Merged

sqlite: add location method#57860
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
geeksilva97:sqlite-add-location-function

Conversation

@geeksilva97

@geeksilva97geeksilva97 commented Apr 13, 2025

Copy link
Copy Markdown
Contributor

Fixes#57859

@nodejs-github-bot

nodejs-github-bot commented Apr 13, 2025

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-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 13, 2025
@geeksilva97

geeksilva97 commented Apr 13, 2025

Copy link
Copy Markdown
ContributorAuthor

This would simplify the sharing of connections between modules. When needed, they could retrieve the database location using only the object.

I'm aware some of you might find this useless. Whatever you guys decide, I'm okay with it. I just needed something like this and decided to implement.

@geeksilva97
geeksilva97force-pushed the sqlite-add-location-function branch from 6de740a to b42a338CompareApril 13, 2025 03:49
@codecov

codecovBot commented Apr 13, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.27%. Comparing base (422529a) to head (0e82ee2).
Report is 4 commits behind head on main.

Files with missing linesPatch %Lines
src/node_sqlite.cc82.60%0 Missing and 4 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #57860 +/- ##
=======================================
Coverage 90.27% 90.27% =======================================
Files 630 630 Lines 186124 186147 +23 Branches 36469 36481 +12 =======================================
+ Hits 168021 168050 +29 + Misses 10974 10972 -2 + Partials 7129 7125 -4
Files with missing linesCoverage Δ
src/node_sqlite.h68.00% <ø> (ø)
src/node_sqlite.cc80.56% <82.60%> (+0.02%)⬆️

... and 25 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.

jasnell
jasnell reviewed Apr 13, 2025
View reviewed changes
Comment threadsrc/node_sqlite.cc Outdated
jasnell
jasnell reviewed Apr 13, 2025
View reviewed changes
Comment threadtest/parallel/test-sqlite-database-sync.js Outdated
anonrig
anonrig reviewed Apr 13, 2025
View reviewed changes
Comment threaddoc/api/sqlite.md Outdated
anonrig
anonrig reviewed Apr 13, 2025
View reviewed changes
Comment threadsrc/node_sqlite.cc Outdated
cjihrig
cjihrig reviewed Apr 13, 2025
View reviewed changes
Comment threadtest/parallel/test-sqlite-database-sync.js Outdated
Comment threaddoc/api/sqlite.md Outdated
Comment threaddoc/api/sqlite.md Outdated
jasnell
jasnell reviewed Apr 13, 2025
View reviewed changes
Comment threadsrc/node_sqlite.cc Outdated
@geeksilva97
geeksilva97force-pushed the sqlite-add-location-function branch 2 times, most recently from f46394b to e7817bfCompareApril 13, 2025 20:59
cjihrig
cjihrig reviewed Apr 13, 2025
View reviewed changes

@cjihrigcjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides the one unrelated change.

Comment threadsrc/node_sqlite.cc Outdated
tniessen
tniessen reviewed Apr 13, 2025
View reviewed changes
Comment threadsrc/node_sqlite.cc Outdated
Comment threadsrc/node_sqlite.cc Outdated
@geeksilva97
geeksilva97force-pushed the sqlite-add-location-function branch 2 times, most recently from 48a8353 to e0418abCompareApril 13, 2025 22:49
jasnell
jasnell reviewed Apr 14, 2025
View reviewed changes
Comment threadsrc/node_sqlite.cc Outdated
@geeksilva97geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 14, 2025

Copy link
Copy Markdown
Collaborator

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

@geeksilva97geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 19, 2025

Copy link
Copy Markdown
Collaborator

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

@geeksilva97
geeksilva97force-pushed the sqlite-add-location-function branch from a489f6f to e68a1b3CompareApril 20, 2025 04:36
@geeksilva97
geeksilva97force-pushed the sqlite-add-location-function branch from e68a1b3 to 0e82ee2CompareApril 21, 2025 01:52
@geeksilva97

geeksilva97 commented Apr 21, 2025

Copy link
Copy Markdown
ContributorAuthor

This has conflicts.

Fixed

@geeksilva97geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 21, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 21, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 21, 2025
@nodejs-github-bot
nodejs-github-bot merged commit e9b286c into nodejs:mainApr 21, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 21, 2025

Copy link
Copy Markdown
Collaborator

Landed in e9b286c

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@geeksilva97@RafaelGSS
sqlite: add location method
b45da5b
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@geeksilva97@RafaelGSS
sqlite: add location method
10eb2b0
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@geeksilva97@aduh95
sqlite: add location method
3bb7085
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@geeksilva97@RafaelGSS
sqlite: add location method
8ade3ce
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@geeksilva97@aduh95
sqlite: add location method
a2f804e
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@geeksilva97@aduh95
sqlite: add location method
95c3785
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@geeksilva97@aduh95
sqlite: add location method
53f4354
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@github-actionsgithub-actionsBot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@geeksilva97@aduh95
sqlite: add location method
7485309
PR-URL: #57860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@geeksilva97
geeksilva97 deleted the sqlite-add-location-function branch May 20, 2025 00:46
@ghostghost mentioned this pull request Jun 8, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonriganonriganonrig left review comments
@tniessentniessentniessen left review comments
@jasnelljasnelljasnell approved these changes
@cjihrigcjihrigcjihrig approved these changes

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a method for retrieving the database location

6 participants

@geeksilva97@nodejs-github-bot@cjihrig@jasnell@anonrig@tniessen