Skip to content

sqlite: set name and length on sqlite.backup() #58251

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

Merged
merged 1 commit into from
May 11, 2025

Conversation

LiviaMedeiros
Copy link
Member

No description provided.

@nodejs-github-bot
Copy link
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 May 9, 2025
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM with a minor nit. Thanks!

return;
}
backup_function->SetName(FIXED_ONE_BYTE_STRING(isolate, "backup"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we already have backup_string that can be used here.

@LiviaMedeiros LiviaMedeiros force-pushed the sqlite-backup-props branch from 1bb0b66 to cc251a3 Compare May 9, 2025 16:27
@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2025
Copy link
Contributor

github-actions bot commented May 9, 2025

Failed to start CI
   ⚠  Commits were pushed since the last approving review:
   ⚠  - sqlite: set `name` and `length` on `sqlite.backup()`
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/14933373676

@github-actions github-actions bot added the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label May 9, 2025
@nodejs-github-bot
Copy link
Collaborator

@LiviaMedeiros LiviaMedeiros removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label May 9, 2025
Copy link

codecov bot commented May 9, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.18%. Comparing base (535c2f7) to head (cc251a3).
Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58251      +/-   ##
==========================================
+ Coverage   90.12%   90.18%   +0.05%     
==========================================
  Files         629      629              
  Lines      186638   186659      +21     
  Branches    36618    36654      +36     
==========================================
+ Hits       168202   168331     +129     
+ Misses      11222    11123      -99     
+ Partials     7214     7205       -9     
Files with missing lines Coverage Δ
src/node_sqlite.cc 80.58% <66.66%> (+0.02%) ⬆️

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

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels May 10, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 11, 2025
@nodejs-github-bot nodejs-github-bot merged commit 770be2c into nodejs:main May 11, 2025
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 770be2c

targos pushed a commit that referenced this pull request May 16, 2025
PR-URL: #58251
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
aduh95 pushed a commit that referenced this pull request Jun 10, 2025
PR-URL: #58251
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

6 participants