Change return value of InsertMany from int64 to more conventional int (breaking) - #293
Merged
Merged
Conversation
Contributor
Author
brandur
force-pushed
the
brandur-insert-many-int
branch
2 times, most recently
from
March 28, 2024 04:57
ac6b7fa to
173e9b0
Compare
bgentry
approved these changes
Mar 29, 2024
bgentry
left a comment
bgentry
left a comment
Contributor
There was a problem hiding this comment.
Needs changelog but LGTM ✌️
brandur
force-pushed
the
brandur-insert-many-int
branch
from
March 29, 2024 04:07
173e9b0 to
af6a891
Compare
Contributor
Author
|
Great! Changelog added, but will hold until we've got all the breaking changes ready. |
brandur
force-pushed
the
brandur-insert-many-int
branch
from
April 16, 2024 01:42
af6a891 to
d9fe9a2
Compare
… `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
force-pushed
the
brandur-insert-many-int
branch
from
April 19, 2024 02:10
d9fe9a2 to
95a0ba4
Compare
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
2 participants
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.
Here, change the type of
Client.InsertManyfunctions fromint64s toa more conventional
int. This isn't really super necessary, but theint64was an artifact of an internal value returned by sqlc, and usingintinstead (which is also an int64 on the vast majority of allsystem) 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 breakvery few integrations in practice. As we can see from our own test
suite, little had to change because
InsertMany's return value israrely needed.