Skip to content

Conversation

@margaretjgu
Copy link
Member

@margaretjgu margaretjgu commented Nov 5, 2025

Closes #778
Original code defined response as an object with no reusable type name, and cannot put @variant container on this object
Refactored to use the value_body pattern (same as SearchResponse)

Does this bug warrant a linting rule? - its a rare and obvious mistake. Maybe a rule is not necessary

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Following you can find the validation changes against the target branch for the APIs.

No changes detected.

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

I think adding a rule for this is not a bad thing to do! I would even go further:

Let's add a rule that explicitly checks if a variant is valid for a given type, e.g.:

Request > none
Response > none
Interface > Container
Alias > Internal, External, Untagged
...

In this particular case the compiler should also throw an exception and refuse emitting the schema.json since this creates a situation that is not mappable to the metamodel. We do this in other situations when a tag is incorrectly used, so this is probably an oversight.

Happy to merge this as is for now. We can improve later :-)

Copy link
Member

@JoshMock JoshMock left a comment

Choose a reason for hiding this comment

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

Code LGTM!

Does this bug warrant a linting rule? - its a rare and obvious mistake. Maybe a rule is not necessary

It caught one bug that you fixed, and is otherwise not going to create any noise. If it ends up causing us pain later we can discuss pulling it out.

@pquentin
Copy link
Member

pquentin commented Nov 7, 2025

Happy to merge this as is for now. We can improve later :-)

Merging, then! @margaretjgu Can you please capture the improvements that Florian identified in a new spec validation issue?

@pquentin pquentin merged commit 1cadc1b into main Nov 7, 2025
10 checks passed
@pquentin pquentin deleted the fix_variant branch November 7, 2025 07:26
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

The backport to 9.1 failed:

The process '/usr/bin/git' failed with exit code 1 

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub git fetch # Create a new working tree git worktree add .worktrees/backport-9.1 9.1 # Navigate to the new working tree cd .worktrees/backport-9.1 # Create a new branch git switch --create backport-5622-to-9.1 # Cherry-pick the merged commit of this pull request and resolve the conflicts git cherry-pick -x --mainline 1 1cadc1b49fc64f0f580a6e981bfdcd6ff8389cb5 # Push it to GitHub git push --set-upstream origin backport-5622-to-9.1 # Go back to the original working tree cd ../.. # Delete the working tree git worktree remove .worktrees/backport-9.1

Then, create a pull request where the base branch is 9.1 and the compare/head branch is backport-5622-to-9.1.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

The backport to 9.2 failed:

The process '/usr/bin/git' failed with exit code 1 

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub git fetch # Create a new working tree git worktree add .worktrees/backport-9.2 9.2 # Navigate to the new working tree cd .worktrees/backport-9.2 # Create a new branch git switch --create backport-5622-to-9.2 # Cherry-pick the merged commit of this pull request and resolve the conflicts git cherry-pick -x --mainline 1 1cadc1b49fc64f0f580a6e981bfdcd6ff8389cb5 # Push it to GitHub git push --set-upstream origin backport-5622-to-9.2 # Go back to the original working tree cd ../.. # Delete the working tree git worktree remove .worktrees/backport-9.2

Then, create a pull request where the base branch is 9.2 and the compare/head branch is backport-5622-to-9.2.

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