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.

Sign up for GitHub

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

Jump to bottom

Upgrade SeaQuery at SeaORM root to v0.27 #985

Merged
merged 16 commits into from
Oct 18, 2022
Merged

Upgrade SeaQuery at SeaORM root to v0.27 #985

merged 16 commits into from
Oct 18, 2022

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 commented Aug 24, 2022 •
edited
Loading

@billy1624 billy1624 self-assigned this Aug 24, 2022
@billy1624 billy1624 mentioned this pull request Aug 24, 2022
Merged
1 task
@billy1624 billy1624 marked this pull request as ready for review August 24, 2022 07:56
@billy1624 billy1624 requested a review from tyt2y3 August 24, 2022 07:56
@billy1624 billy1624 changed the title Upgrade SeaORM root's SeaQuery to v0.27 Upgrade SeaQuery at SeaORM root to v0.27 Aug 26, 2022
@ikrivosheev
Copy link
Member

@billy1624, hello! I think we can combine your PR and mine PR: #970

@billy1624
Copy link
Member Author

Agree!

@billy1624 billy1624 force-pushed the sea-query-v0.27 branch 2 times, most recently from 5f97cb5 to 7e46e23 Compare September 22, 2022 07:06
@billy1624 billy1624 marked this pull request as draft September 22, 2022 07:16
@ikrivosheev
Copy link
Member

@billy1624 hello! Why didn't we use ColumnDef and ColumnType from sea-query?

@tyt2y3
Copy link
Member

tyt2y3 commented Sep 27, 2022 •
edited
Loading

This is a good question. The primary motivation is SeaQuery integer types have the optional (and meaningless and deprecated) column width parameter , i.e. integer(8). And I wanted to hide these from the SeaORM API. So the right thing to do might be to remove those from SeaQuery.

The ethos is, SeaORM is higher-level and so should provide some reasonable defaults.

@ikrivosheev ikrivosheev mentioned this pull request Oct 17, 2022
Closed
@billy1624 billy1624 mentioned this pull request Oct 17, 2022
Merged
1 task
@billy1624 billy1624 mentioned this pull request Oct 17, 2022
Merged
1 task
@tyt2y3
Copy link
Member

tyt2y3 commented Oct 17, 2022

Definitely add some test cases for sqlx-postgres-array!

@billy1624
Copy link
Member Author

Definitely add some test cases for sqlx-postgres-array!

I will add that in a separate PR. Let's focus on bumping SeaQuery version on this PR.

@billy1624 billy1624 marked this pull request as ready for review October 18, 2022 12:15
@billy1624
Copy link
Member Author

Hey @tyt2y3, ready for review as well.

tyt2y3
tyt2y3 approved these changes Oct 18, 2022
View reviewed changes
Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

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

Thank you both of you!

@tyt2y3 tyt2y3 merged commit ef14412 into master Oct 18, 2022
@tyt2y3 tyt2y3 deleted the sea-query-v0.27 branch October 18, 2022 16:16
@tyt2y3
Copy link
Member

tyt2y3 commented Oct 18, 2022

Strange, there are still failure https://github.com/SeaQL/sea-orm/actions/runs/3275174232/jobs/5389786280

@billy1624
Copy link
Member Author

We need to publish a new version for sea-query-binder as well. Since we also changed the binder.

@Razican
Copy link

Razican commented Oct 19, 2022

I noticed that if you use the ipnetwork or the mac_address feature in sea-query-binder, you get an error in sea-Tom, because they are not supported. Is this a known issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyt2y3 tyt2y3 tyt2y3 approved these changes

Assignees

@billy1624 billy1624

Labels
None yet
Projects
SeaQL Dev Tracker
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support sea-query-binder
4 participants
@billy1624 @ikrivosheev @tyt2y3 @Razican