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

feat(stackable-versioned): Add support for type changes of fields #844

Merged
merged 15 commits into from
Sep 4, 2024

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Aug 21, 2024

Part of stackabletech/issues#507

This PR adds support for type changes of struct fields. This is done by renaming the renamed() action to changed() and adding a new parameter from_type to it. Additionally, from will be renamed to from_name.

Reviewer

@Techassi Techassi force-pushed the feat/crd-versioning-type-change branch from a1fd53f to 0bfa8da Compare August 30, 2024 12:18
The current implementation is very brittle and less
than ideal. It currently only produces correct code
in the struct definitions, but not in the From impls.
For that reason, test files currently skip the From
impl generation.

I have a bunch of thoughts on how to improve the
situation, but I would like to tackle these changes
in follow-up commits.
@Techassi Techassi force-pushed the feat/crd-versioning-type-change branch from f2c367e to 118ed09 Compare September 2, 2024 10:01
@Techassi
Copy link
Member Author

Techassi commented Sep 3, 2024

I would like to adjust some of the internal types and how the validation works. These changes are however not included in this PR, because it proved to be very difficult to implement the changes at the current moment. I will tackle this in future PRs. Also see TedDriggs/darling#304.

It also keeps the PR small and focuses on the type change feature.

@Techassi Techassi marked this pull request as ready for review September 3, 2024 12:42
NickLarsenNZ
NickLarsenNZ previously approved these changes Sep 3, 2024
Copy link
Member

@NickLarsenNZ NickLarsenNZ left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@NickLarsenNZ NickLarsenNZ left a comment

Choose a reason for hiding this comment

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

LGTM

@Techassi Techassi added this pull request to the merge queue Sep 4, 2024
Merged via the queue into main with commit 494ed37 Sep 4, 2024
20 checks passed
@Techassi Techassi deleted the feat/crd-versioning-type-change branch September 4, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants