Skip to content

Conversation

@tzador
Copy link
Collaborator

@tzador tzador commented Oct 31, 2025

No description provided.

@tzador tzador requested a review from Cahllagerfeld October 31, 2025 13:55
.min(1, "Stack name is required")
.max(255, "Stack name must be less than 255 characters")
.refine((name) => validateStackName(name), "Stack name is already in use");
.max(32, "Stack name must be less than 32 characters")
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you come to the conclusion of this?

grafik

I just checked the Swagger-Docs and the only restriction is the length of 255 characters that we had before 🤔

Copy link
Collaborator Author

@tzador tzador Oct 31, 2025

Choose a reason for hiding this comment

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

In the designs it was 32, but yes, api is king, so back to 255

.max(255, "Stack name must be less than 255 characters")
.refine((name) => validateStackName(name), "Stack name is already in use");
.max(32, "Stack name must be less than 32 characters")
.refine((name) => validateStackNameNotInUse(name), "Stack name is already in use")
Copy link
Contributor

Choose a reason for hiding this comment

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

As sending the request is the most expensive operation during the validation, we should run it as the last validator. Imo only after all the other checks were good, we should run it. Wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, swapped the order, not sure how much it improves, but if earlier rule fails first, it will show up sooner.

@tzador
Copy link
Collaborator Author

tzador commented Oct 31, 2025

PTAL

@tzador tzador requested a review from Cahllagerfeld November 4, 2025 08:44
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.

3 participants