Skip to content

fix(stackable-versioned): Emit right type for unchanged fields #860

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

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Sep 6, 2024

Part of stackabletech/issues#507

Previously, the type used in fields marked as NoChange in a particular version was incorrect. It used the type of the definition container. The NoChange action now also tracks the field type and can thus generate the correct type.

Discovered by @sbernauer.

Previously, the type used in fields marked as
NoChange in a particular version was incorrect. It
used the type of the definition container. The
NoChange action now also tracks the field type and
can thus generate the correct type.
@Techassi Techassi self-assigned this Sep 6, 2024
@Techassi Techassi marked this pull request as ready for review September 6, 2024 14:37
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. I assume the "good" trybuild validates this (since the added fields was removed).

@Techassi
Copy link
Member Author

Techassi commented Sep 9, 2024

LGTM. I assume the "good" trybuild validates this (since the added fields was removed).

Yes exactly. Previously, the faulty behaviour was hidden by the superfluous added action. By removing it, we purposely trigger the edge-case where the issue occurred.

@Techassi Techassi added this pull request to the merge queue Sep 9, 2024
Merged via the queue into main with commit 1b8c400 Sep 9, 2024
10 checks passed
@Techassi Techassi deleted the fix/crd-versioning-track-type-nochange branch September 9, 2024 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants