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

Change return value of InsertMany from int64 to more conventional int (breaking) #293

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Mar 28, 2024

Here, change the type of Client.InsertMany functions from int64s to
a more conventional int. This isn't really super necessary, but the
int64 was an artifact of an internal value returned by sqlc, and using
int instead (which is also an int64 on the vast majority of all
system) is more conventional.

Because callers almost always know the size of the slice they passed to
InsertMany, or can ascertain it easily, I expect this change to break
very few integrations in practice. As we can see from our own test
suite, little had to change because InsertMany's return value is
rarely needed.

@brandur brandur added the breaking change Breaking API change label Mar 28, 2024
@brandur
Copy link
Contributor Author

brandur commented Mar 28, 2024

To ship out alongside #236 and #292 in a single new version containing multiple minor breaking changes.

@brandur brandur force-pushed the brandur-insert-many-int branch 2 times, most recently from ac6b7fa to 173e9b0 Compare March 28, 2024 04:57
Copy link
Contributor

@bgentry bgentry left a comment

Choose a reason for hiding this comment

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

Needs changelog but LGTM ✌️

@brandur brandur force-pushed the brandur-insert-many-int branch from 173e9b0 to af6a891 Compare March 29, 2024 04:07
@brandur
Copy link
Contributor Author

brandur commented Mar 29, 2024

Great! Changelog added, but will hold until we've got all the breaking changes ready.

@brandur brandur force-pushed the brandur-insert-many-int branch from af6a891 to d9fe9a2 Compare April 16, 2024 01:42
… `int` (breaking)

Here, change the type of `Client.InsertMany` functions from `int64`s to
a more conventional `int`. This isn't really super necessary, but the
`int64` was an artifact of an internal value returned by sqlc, and using
`int` instead (which is also an int64 on the vast majority of all
system) is more conventional.

Because callers almost always know the size of the slice they passed to
`InsertMany`, or can ascertain it easily, I expect this change to break
very few integrations in practice. As we can see from our own test
suite, little had to change because `InsertMany`'s return value is
rarely needed.
@brandur brandur force-pushed the brandur-insert-many-int branch from d9fe9a2 to 95a0ba4 Compare April 19, 2024 02:10
@brandur brandur merged commit 78a2245 into master Apr 19, 2024
10 checks passed
@brandur brandur deleted the brandur-insert-many-int branch April 19, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaking API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants