Skip to content

Conversation

psyclaudeZ
Copy link
Contributor

@psyclaudeZ psyclaudeZ commented Apr 3, 2025

Before this PR

When an empty union is defined, conjure-python will incorrectly generate code with syntax error like:

        if type_of_union is None:
            if  != 1:
                raise ValueError('a union must contain a single member')

After this PR

For backward compat, we have to handle the scenario post-hoc, i.e. after codegen. The wire spec doesn't explicitly state whether an empty union is allowed, but here I'm just throwing an exception during runtime anyway (it won't break existing generated binding per se - since they should be already broken by the syntax error) since relaxation is easier than tightening up if there's a change of plan.

==COMMIT_MSG==
Handle empty union type
==COMMIT_MSG==

Possible downsides?

@psyclaudeZ psyclaudeZ requested a review from johnhany97 April 3, 2025 08:32
Copy link

stale bot commented Jun 27, 2025

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Jun 27, 2025
@psyclaudeZ psyclaudeZ requested a review from BenjaminLowry June 27, 2025 06:55
@stale stale bot removed the stale label Jun 27, 2025
@psyclaudeZ
Copy link
Contributor Author

Does this make sense @BenjaminLowry ? (Consider hiding whitespace to unclutter the view)

Copy link
Contributor

@BenjaminLowry BenjaminLowry left a comment

Choose a reason for hiding this comment

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

Yeah this makes sense to me

@bulldozer-bot bulldozer-bot bot merged commit 914838c into develop Aug 1, 2025
7 checks passed
@bulldozer-bot bulldozer-bot bot deleted the psyclaudeZ/union branch August 1, 2025 22:13
@autorelease3
Copy link

autorelease3 bot commented Aug 1, 2025

Released 4.22.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants