Skip to content

Conversation

@gwynne
Copy link
Member

@gwynne gwynne commented Nov 11, 2022

The sqlite3_libversion() and sqlite3_libversion_number() APIs are now wrapped by public static Swift methods on SQLConnection, making it simpler for clients to find out what version is in use. Provides necessary support for a corresponding update to enable version-specific feature support in vapor/sqlite-kit.

Additional changes:

  • Dropped support for Swift versions up to and including 5.4; 5.5 is now the minimum.
  • CI has been heavily updated (current Swift versions, code coverage and API breakage, etc.)
  • Modernized use of NIO by unit tests.
  • Allow overriding logging level in tests with LOG_LEVEL env var.

@gwynne gwynne added enhancement New feature or request semver-minor Contains new APIs labels Nov 11, 2022
@gwynne gwynne requested a review from 0xTim November 11, 2022 22:39
@gwynne gwynne self-assigned this Nov 11, 2022
@gwynne gwynne force-pushed the version-reporting-support branch from 0ac5d5e to 369674e Compare November 11, 2022 23:05
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@a9a862d). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 0ac5d5e differs from pull request most recent head 369674e. Consider uploading reports for the commit 369674e to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #34   +/-   ##
=======================================
  Coverage        ?   45.68%           
=======================================
  Files           ?        7           
  Lines           ?      510           
  Branches        ?        0           
=======================================
  Hits            ?      233           
  Misses          ?      277           
  Partials        ?        0           
Flag Coverage Δ
unittests 45.68% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

This looks good overall. Main comment is that there no test to ensure the new API can at least be called. And will be this superseded by #35 if we can just control the version we embed

@gwynne
Copy link
Member Author

gwynne commented Nov 12, 2022

This looks good overall. Main comment is that there no test to ensure the new API can at least be called. And will be this superseded by #35 if we can just control the version we embed

I thought about adding a test, but the most it could possibly verify is SQLConnection.versionNumber == sqlite3_libversion(), which is so thoroughly redundant as to be pointless. The API still has legitimate use after #35 is merged; we don't want to hardcode things into SQLiteDialect just 'cause we can (especially since users might presumably need to roll back to an earlier SQLiteNIO - for example, due to bugs in a new SQLite release). The API is also desirable to have generally available for clients of this layer to use (i.e. rather than having to import SQLite).

@gwynne gwynne requested a review from 0xTim November 12, 2022 11:22
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Cool ship it!

@gwynne gwynne merged commit f68a2bc into main Nov 12, 2022
@gwynne gwynne deleted the version-reporting-support branch November 12, 2022 11:59
@VaporBot
Copy link

These changes are now available in 1.2.0

gwynne added a commit to danramteke/sqlite-nio that referenced this pull request Feb 22, 2023
* Drop support for Swift <5.5

* Bring CI up to date

* Modernize NIO usage and add log level override support for unit tests

* Add static methods on SQLiteConnection to make the runtime version of SQLite available to higher layers without having to use SQLite APIs directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request semver-minor Contains new APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants