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

Implement "Nest" for DerivePartialModel and From QueryResult #1716

Closed
wants to merge 15 commits into from

Conversation

Goodjooy
Copy link
Contributor

@Goodjooy Goodjooy commented Jun 21, 2023

PR Info

New Features

  • FromQueryResult support flatten
  • DerivePartialModel support flatten
  • document update
  • test case

Changes

  • FromQueryResult has attributes sea_orm

@Goodjooy Goodjooy marked this pull request as ready for review June 21, 2023 13:40
@tyt2y3
Copy link
Member

tyt2y3 commented Jun 22, 2023

Thank you for the contribution again! You might be a stronger derive macro guru than I am.
Hopefully this will not conflict with the effort "upgrade to syn2" #1713

@tyt2y3 tyt2y3 requested a review from billy1624 June 22, 2023 07:58
@Goodjooy
Copy link
Contributor Author

Thank you for the contribution again! You might be a stronger derive macro guru than I am. Hopefully this will not conflict with the effort "upgrade to syn2" #1713

Crate syn with its major version update usually contain one or more break changes ,I cannot guarantee there is no conflict.

I think I can wait for that PR merged ,then I will handle the conflict in this PR.

@tyt2y3
Copy link
Member

tyt2y3 commented Sep 9, 2023

Two things:

  1. the implementation looks nice!
  2. can you add an example or test case showing how the flatten should work?
  3. what do those icon in front of the commit messages mean? is there a translation table? 👀

@Goodjooy
Copy link
Contributor Author

Goodjooy commented Sep 9, 2023

Two things:

1. the implementation looks nice!

2. can you add an example or test case showing how the flatten should work?

3. what do those icon in front of the commit messages mean? is there a translation table? 👀

2 things but 3 points :)

The emoji is 'gitmoji' I like using those emoji to mark each commit action.
this is the webside of https://gitmoji.dev/

I'll add examples in the near future.

# Conflicts:
#	sea-orm-macros/src/derives/from_query_result.rs
@jreppnow
Copy link

(Sorry for coming in from the side..)

Is there a specific reason this was closed? We were looking forward to using this - if required, I can also take a shot at implementing the nesting bit.

@Goodjooy
Copy link
Contributor Author

(Sorry for coming in from the side..)

Is there a specific reason this was closed? We were looking forward to using this - if required, I can also take a shot at implementing the nesting bit.

Why this pr closed? I had never close it?

@Goodjooy Goodjooy reopened this Mar 19, 2024
@Goodjooy Goodjooy marked this pull request as draft March 19, 2024 16:21
@Goodjooy
Copy link
Contributor Author

However, the code is a bit old, I need time to read and refactor it

@jreppnow
Copy link

jreppnow commented Mar 29, 2024

@Goodjooy Hi again、I am really sorry for coming in from the side, but since we kind of desperately need this feature, I decided to do a separate implementation in #2179 . I honestly only care about this feature landing as quickly as possible, so I would not mind if you take my code and update your PR, if you prefer that path instead of my PR superseding yours.

CC: @tyt2y3

@Goodjooy
Copy link
Contributor Author

@Goodjooy Hi again、I am really sorry for coming in from the side, but since we kind of desperately need this feature, I decided to do a separate implementation in #2179 . I honestly only care about this feature landing as quickly as possible, so I would not mind if you take my code and update your PR, if you prefer that path instead of my PR superseding yours.

CC: @tyt2y3

Maintaining your own modified version of the nest feature might be a more ideal way right now. Adding features usually needs a minor version update which might waiting a bit longer than bug fixing. Not to mention that the core maintainers now are busy in releasing the 1.0 version right now.

@tyt2y3
Copy link
Member

tyt2y3 commented Mar 29, 2024

Hey, thank you for everyone's involvement. I think the only things missing from this PR are some tests and a bit of doc example?

@Goodjooy
Copy link
Contributor Author

Goodjooy commented Apr 1, 2024

there was a better one #2179 that this pr can be close

@Goodjooy Goodjooy closed this Apr 1, 2024
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