Skip to content

Conversation

tyt2y3
Copy link
Member

@tyt2y3 tyt2y3 commented Aug 17, 2025

I am slowly writing it now

@tyt2y3
Copy link
Member Author

tyt2y3 commented Aug 17, 2025

@Huliiiiii @Expurple

Copy link

github-actions bot commented Aug 17, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://www.sea-ql.org/preview/pr-150/

Built to branch gh-pages at 2025-08-26 09:20 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Comment on lines 16 to 22
## Architectural changes

There are a few architectural changes that can only be made by breaking the API, so let's go through them one by one:

### Forbid unsafe code

#![forbid(unsafe_code)] has been added to all workspace crates, ensuring that SeaQuery no longer contains any unsafe code. While only one instance of unsafe was previously used, and has now been removed, this change reinforces our commitment to maintaining code quality.
Copy link
Member

Choose a reason for hiding this comment

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

This reads a little weird because #![forbid(unsafe_code)] isn't a breaking change for the users (or a big internal architectural change, really). But it's still good to advertize in front of the list. Maybe do it like this?

Suggested change
## Architectural changes
There are a few architectural changes that can only be made by breaking the API, so let's go through them one by one:
### Forbid unsafe code
#![forbid(unsafe_code)] has been added to all workspace crates, ensuring that SeaQuery no longer contains any unsafe code. While only one instance of unsafe was previously used, and has now been removed, this change reinforces our commitment to maintaining code quality.
## Forbid unsafe code
#![forbid(unsafe_code)] has been added to all workspace crates, ensuring that SeaQuery no longer contains any unsafe code. While only one instance of unsafe was previously used, and has now been removed, this change reinforces our commitment to maintaining code quality.
## Architectural changes
There are a few architectural changes that can only be made by breaking the API, so let's go through them one by one:

Or we can just keep it as is.

@tyt2y3 tyt2y3 marked this pull request as ready for review August 24, 2025 23:36
@tyt2y3
Copy link
Member Author

tyt2y3 commented Aug 24, 2025

Copy link
Member

@Expurple Expurple left a comment

Choose a reason for hiding this comment

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

LGTM. I only need to finish with the remaining Into* traits and update the list here

Comment on lines 469 to 473
* `IntoCondition` [#939](https://github.com/SeaQL/sea-query/pull/939)
* `IntoColumnRef` [#959](https://github.com/SeaQL/sea-query/pull/959)
* `IntoTableRef` [#958](https://github.com/SeaQL/sea-query/pull/958)
* `IntoTypeRef` [#969](https://github.com/SeaQL/sea-query/pull/969)
* `IntoValueTuple` [#960](https://github.com/SeaQL/sea-query/pull/960)
Copy link
Member

Choose a reason for hiding this comment

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

Done with the Into* traits.

Although, now I also want to experiment with removing Expr::TypeName before the release: SeaQL/sea-query#954 (comment)

Suggested change
* `IntoCondition` [#939](https://github.com/SeaQL/sea-query/pull/939)
* `IntoColumnRef` [#959](https://github.com/SeaQL/sea-query/pull/959)
* `IntoTableRef` [#958](https://github.com/SeaQL/sea-query/pull/958)
* `IntoTypeRef` [#969](https://github.com/SeaQL/sea-query/pull/969)
* `IntoValueTuple` [#960](https://github.com/SeaQL/sea-query/pull/960)
* `IntoColumnDef` [#975](https://github.com/SeaQL/sea-query/pull/975)
* `IntoColumnRef` [#959](https://github.com/SeaQL/sea-query/pull/959)
* `IntoCondition` [#939](https://github.com/SeaQL/sea-query/pull/939)
* `IntoIden` [#973](https://github.com/SeaQL/sea-query/pull/973)
* `IntoIndexColumn` [#976](https://github.com/SeaQL/sea-query/pull/976)
* `IntoLikeExpr` [#974](https://github.com/SeaQL/sea-query/pull/974)
* `IntoTableRef` [#958](https://github.com/SeaQL/sea-query/pull/958)
* `IntoTypeRef` [#969](https://github.com/SeaQL/sea-query/pull/969)
* `IntoValueTuple` [#960](https://github.com/SeaQL/sea-query/pull/960)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added! many thanks

Copy link
Member Author

@tyt2y3 tyt2y3 Aug 27, 2025

Choose a reason for hiding this comment

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

Although, now I also want to experiment with removing Expr::TypeName before the release

it should be fine, we're still in rc. oh we missed this week's this-week-in-rust. let's publish it next week then.

@tyt2y3 tyt2y3 merged commit 5ae0014 into master Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants