Open
Conversation
…allow non-LTO linking
…el/ScalableVectorSearch into rfsaliev/cpp-runtime-binding
…el/ScalableVectorSearch into rfsaliev/cpp-runtime-binding
…el/ScalableVectorSearch into rfsaliev/cpp-runtime-binding
…ntel/ScalableVectorSearch into feature_test_vesrion_namespace
5cb9050 to
f2680de
Compare
9df4e53 to
ee0168d
Compare
ahuber21
requested changes
Dec 2, 2025
Contributor
ahuber21
left a comment
There was a problem hiding this comment.
Sorry, forgot to submit. Could you rebase the branch first, then we can discuss the details.
|
|
||
| CATCH_SECTION("Version string") { | ||
| // Verify version string is accessible | ||
| CATCH_REQUIRE(svs_runtime::VersionInfo::get_version() != nullptr); |
Contributor
There was a problem hiding this comment.
What about checking the values?
| #endif | ||
|
|
||
| // Create namespace alias (FAISS integration pattern) | ||
| SVS_RUNTIME_CREATE_API_ALIAS(svs_runtime, FAISS_SVS_RUNTIME_VERSION); |
Contributor
There was a problem hiding this comment.
We can pick a more generic name here
Suggested change
| SVS_RUNTIME_CREATE_API_ALIAS(svs_runtime, FAISS_SVS_RUNTIME_VERSION); | |
| SVS_RUNTIME_CREATE_API_ALIAS(svs_runtime, RUNTIME_API_VERSION); |
Contributor
There was a problem hiding this comment.
Maybe also add a test that uses anything beyond version info from the runtime, like svs_runtime::VamanaIndex?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add simple version namespace usecase, also added in ci to run this c++ test
orig version namespace commit
a551e9ba8bc232abf4ac5e11e01a85b9b403e6e2
bcd7cb3