Skip to content
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

sqlite: refactor open options #55442

Merged

Conversation

tniessen
Copy link
Member

Move options that are only relevant for opening the database into a self-contained class.

Refs: #54777 (comment)

Move options that are only relevant for opening the database into a
self-contained class.
@tniessen tniessen added the sqlite Issues and PRs related to the SQLite subsystem. label Oct 18, 2024
@tniessen tniessen requested a review from jasnell October 18, 2024 09:54
@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. labels Oct 18, 2024
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.40%. Comparing base (ddfef05) to head (dc87f5c).
Report is 63 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55442      +/-   ##
==========================================
- Coverage   88.42%   88.40%   -0.02%     
==========================================
  Files         652      653       +1     
  Lines      186918   187605     +687     
  Branches    36079    36120      +41     
==========================================
+ Hits       165279   165851     +572     
- Misses      14892    14979      +87     
- Partials     6747     6775      +28     
Files with missing lines Coverage Δ
src/node_sqlite.h 73.33% <100.00%> (+73.33%) ⬆️
src/node_sqlite.cc 83.36% <87.50%> (-0.37%) ⬇️

... and 104 files with indirect coverage changes

@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2024
@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 24, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 24, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/55442
✔  Done loading data for nodejs/node/pull/55442
----------------------------------- PR info ------------------------------------
Title      sqlite: refactor open options (#55442)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:sqlite-refactor-open-options -> nodejs:main
Labels     c++, needs-ci, sqlite
Commits    1
 - sqlite: refactor open options
Committers 1
 - Tobias Nießen <tniessen@tnie.de>
PR-URL: https://github.com/nodejs/node/pull/55442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 18 Oct 2024 09:54:50 GMT
   ✔  Approvals: 1
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/55442#pullrequestreview-2377857073
   ✘  This PR needs to wait 10 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-24T17:37:49Z: https://ci.nodejs.org/job/node-test-pull-request/63277/
- Querying data for job/node-test-pull-request/63277/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/11509076184

@tniessen tniessen added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 26, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 26, 2024
@nodejs-github-bot nodejs-github-bot merged commit 0668e64 into nodejs:main Oct 26, 2024
80 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 0668e64

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2024
Move options that are only relevant for opening the database into a
self-contained class.

PR-URL: #55442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
Move options that are only relevant for opening the database into a
self-contained class.

PR-URL: nodejs#55442
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
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. sqlite Issues and PRs related to the SQLite subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants