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

Support --column-extra-derives #2212

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

karolgorecki
Copy link

@karolgorecki karolgorecki commented May 1, 2024

PR Info

Doesn't close any issue.
Functionally related to, but not overlapping with: #1934 & #1952

New Features

  • New --column-extra-derives CLI option allows for adding custom derives to column enum.

Bug Fixes

None.

Breaking Changes

Fully backward compatible for the end user.

Changes

None.

@karolgorecki karolgorecki changed the title Support --column-extra-derives Support --column-extra-derives May 1, 2024
Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

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

Hey @karolgorecki, thanks for contributing!!

I think this PR is incomplete? I saw the codegen writer for the compact entity format is still not implemented. I will convert this as a draft PR :)

@billy1624
Copy link
Member

Do let me know if you need help!

@billy1624 billy1624 marked this pull request as draft May 2, 2024 07:21
@karolgorecki
Copy link
Author

karolgorecki commented May 2, 2024

Hi @billy1624 for the compact entity format there is no column enum so it's passed but not used. We could also not pass it but then will have to change the assert_serde_variant_results

@karolgorecki karolgorecki marked this pull request as ready for review May 2, 2024 10:55
@karolgorecki
Copy link
Author

Hi @billy1624 is it ok or I should not pass it + update the assert_serde_variant_results?

@karolgorecki karolgorecki marked this pull request as draft May 23, 2024 08:02
@karolgorecki karolgorecki marked this pull request as ready for review May 23, 2024 08:02
@karolgorecki
Copy link
Author

Hi @billy1624 can you check again the PR?

@funlennysub
Copy link

Any plans on adding this in the near future? Would be nice to have instead of manually writing PartialEq impl's for the columns enum.

@karolgorecki
Copy link
Author

@billy1624 ?

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