Skip to content

fix(schema): Implement remove other for enums#5962

Open
Dav1dde wants to merge 2 commits intomasterfrom
dav1d/impl-remove-other-for-enums
Open

fix(schema): Implement remove other for enums#5962
Dav1dde wants to merge 2 commits intomasterfrom
dav1d/impl-remove-other-for-enums

Conversation

@Dav1dde
Copy link
Copy Markdown
Member

@Dav1dde Dav1dde commented May 8, 2026

Apparently this was never implemented for ProcessValue, I think I am calling the right enter/exit functions, but would appreciated a second set of eyes and thoughts whether this is actually correct.

One big side-track trying to implement #5280

@Dav1dde Dav1dde self-assigned this May 8, 2026
@Dav1dde Dav1dde requested a review from a team as a code owner May 8, 2026 10:59
@Dav1dde Dav1dde force-pushed the dav1d/impl-remove-other-for-enums branch from 0e1ef81 to a01284d Compare May 8, 2026 11:01
@Dav1dde Dav1dde force-pushed the dav1d/impl-remove-other-for-enums branch from a01284d to d2f8127 Compare May 8, 2026 11:01
@Dav1dde Dav1dde marked this pull request as draft May 8, 2026 11:24
Copy link
Copy Markdown
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

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

Is there more to do? So far this looks extremely reasonable.

@Dav1dde Dav1dde marked this pull request as ready for review May 8, 2026 11:55
@Dav1dde
Copy link
Copy Markdown
Member Author

Dav1dde commented May 8, 2026

@loewenheim put it back in draft because I realized retain wasn't handled correctly and the change would've deleted custom contexts from users.

Annotated all uses of fallback_variant (only 2) with retain, just to make it explicit.

@Dav1dde Dav1dde requested a review from loewenheim May 8, 2026 11:56
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.

2 participants