Merged
Conversation
Member
|
Looks like |
hermanschaaf
approved these changes
Nov 7, 2022
Contributor
hermanschaaf
left a comment
There was a problem hiding this comment.
I actually ended up reviewing the other PR first in any case 👍 I'll leave it up to you how you want to merge the changes.
Contributor
Author
Thanks! I'll go with that first and then open a new one on top as I ended up making it backward compatible and not breaking the protocol. |
kodiakhq bot
pushed a commit
that referenced
this pull request
Nov 7, 2022
🤖 I have created a release *beep* *boop* --- ## [0.13.23](v0.13.22...v0.13.23) (2022-11-07) ### Bug Fixes * Move cqtypes ([#357](#357)) ([9064bc0](9064bc0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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
3 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.
This is a split of this PR #348 so it will be easier to review.
Also, Im going to push the move to v2 maybe another week as we dont really have anything that requires protocol changes for now. even though it is the right way to do it and will make the cli much easier including the need to things like that - cloudquery/cloudquery#3657 as it will be all handles on protobuf and go versioning side.
This current PR just moves cqtypes to schema and removes enumerating all types in different locations to make sure when we add a new type tests will catch if a type is not added to the right place and or not connected to
TypeValue.Also this removes
TimeIntervalwhich 1) wasn't used 2) it is incorrect because it usedtime.DurationasTimeIntervalbut it's not a TimeInterval it's just a duration which is uint64 so there no need for a special type for that. In PostgreSQL TimeInterval is actually completely different type that contains timezones and it is not gotime.Duration