-
Notifications
You must be signed in to change notification settings - Fork 100
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
[Format] Document API/ABI stability goals #96
Conversation
2e5b7be
to
9c2e329
Compare
I suppose, @pitrou @kou @hannes if you have thoughts about how to achieve ABI compatibility/on the two approaches above - AFAIK this is the last "spec" thing before I turn my focus fully towards implementation (except for @zeroshade adding some option definitions for transaction isolation levels in #97) |
+1 |
I do not know what kind of changes may be expected in the future so I have no idea which approach would be better. Wording-wise, "add and remove new functions" sounds weird - how do you remove new functions? |
Whoops, that should be "add new functions and remove existing ones" - will adjust |
9c2e329
to
fe07704
Compare
Assuming no more comments, I'll update this to reflect the versioning strategy Kou proposed, and then merge this |
fe07704
to
303a26c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
FILL_DEFAULT(driver, StatementSetOption); | ||
FILL_DEFAULT(driver, StatementSetSqlQuery); | ||
FILL_DEFAULT(driver, StatementSetSubstraitPlan); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we return a non-ADBC_STATUS_OK
status if version != ADBC_VERSION_1_0_0
here?
@lidavidm How frequently do we expect to be making backwards-incompatible changes that will bump the major version number? Specifically, I'm wondering if we should start with the Go package being For everything else, this LGTM |
I would think fairly infrequently in aggregate, but I wouldn't be surprised if a v2 comes relatively quickly once we get some real-world feedback. Same with Flight: most of what was there originally didn't change, but several things got patched or added quickly, and even still there's 1 or 2 more things we probably want to add (though not things that are critical to do right now) |
Okay, then I'll leave the v1 out of the import path for now until we smooth out a full release process and such and we can add the btw: I don't know if we want to have this before we do the v1 release or not, but I'll be putting up a PR tomorrow which will include a Go wrapper for the C adbc driver manager via CGO. |
Based on the versioning scheme we can have a 1.0.0 API spec without the 1.0.0 libraries quite yet ready so I think there's no rush |
303a26c
to
b35d2b8
Compare
Fixes #77.