Conversation
There was a problem hiding this comment.
Code Review
This pull request increases the Mocha test timeout and implements stricter validation for backend IDs, including a minimum length of three characters and a specific naming format. A review comment suggests refactoring the validation logic in src/apphosting/backend.ts to improve readability by breaking down the complex regular expression into multiple simpler checks.
| validate: (s) => { | ||
| if (!/^[a-z](?:[a-z0-9-]*[a-z0-9])?$/.test(s)) { | ||
| return "Must begin with a letter, can contain only lowercase, digits, hyphens, and cannot end with hyphen"; | ||
| } else if (s.length < 3 || s.length > 30) { | ||
| return "Must be between 3 and 30 characters"; | ||
| } | ||
|
|
||
| return true; | ||
| }, |
There was a problem hiding this comment.
While the regular expression is correct, it's a bit complex and can be hard to parse for future maintainers. For better readability and maintainability, you could split the validation into multiple, simpler checks. This makes the intent of each validation step clearer and is generally a good practice.
validate: (s) => {
if (s.length < 3 || s.length > 30) {
return "Must be between 3 and 30 characters";
}
if (!/^[a-z]/.test(s) || /[^a-z0-9-]/.test(s) || s.endsWith("-")) {
return "Must begin with a letter, can contain only lowercase, digits, hyphens, and cannot end with hyphen";
}
return true;
},
Description
Add more validation to the backend id prompt. based on the network responses from providing invalid backend ids
Scenarios Tested
All conditions followed
Sample Commands