Skip to content

Conversation

@MattDevy
Copy link
Contributor

As per #5642 adds checks for variant types allowed in specific cases

@MattDevy MattDevy requested a review from margaretjgu November 14, 2025 16:23
@MattDevy MattDevy added skip-backport This pull request should not be backported spec validation and removed specification labels Nov 14, 2025
@github-actions
Copy link
Contributor

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.

| `invalid-node-types` | The spec uses a subset of TypeScript, so some types, clauses and expressions are not allowed. |
| `no-generic-number` | Generic `number` type is not allowed outside of `_types/Numeric.ts`. Use concrete numeric types like `integer`, `long`, `float`, `double`, etc. |
| `request-must-have-urls` | All Request interfaces extending `RequestBase` must have a `urls` property defining their endpoint paths and HTTP methods. |
| `no-variants-on-responses` | `@variants` is only supported on Interface types, not on Request or Response classes. Use value_body pattern with `@codegen_name` instead. |
Copy link
Member

Choose a reason for hiding this comment

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

lgtm! Although I think it would be nice to extend off of the existing variant rule...at least thats what I had in mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make a lot of sense. I will refactor it into the existing rule Monday morning! 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@margaretjgu refactored in e414575 :)

@MattDevy MattDevy requested a review from margaretjgu November 17, 2025 14:11
@pquentin pquentin added backport 9.1 backport 9.2 and removed skip-backport This pull request should not be backported labels Nov 20, 2025
@pquentin pquentin merged commit 207103e into main Nov 20, 2025
15 checks passed
@pquentin pquentin deleted the chore/check-variants branch November 20, 2025 13:01
@github-actions
Copy link
Contributor

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-5678-to-9.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 207103e76aa757a2b2a0fb9f576121d90da823a2
# Push it to GitHub
git push --set-upstream origin backport-5678-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-5678-to-9.1.

@github-actions
Copy link
Contributor

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-5678-to-9.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 207103e76aa757a2b2a0fb9f576121d90da823a2
# Push it to GitHub
git push --set-upstream origin backport-5678-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-5678-to-9.2.

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.

4 participants