-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate entity names locally #3296
Conversation
richard-cox
commented
Jan 2, 2019
- validate length of entity names during create process
- service instances, route host and route path
- app, org and space seem to handle long names fine
- fixes Service instance names should be limited to 50 chars #3262
- service instances, route host and route path - app, org and space seem to handle long names fine
✅ Hey richard-cox! The commit authors and yourself have already signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## v2-master #3296 +/- ##
=============================================
- Coverage 70.77% 70.75% -0.02%
=============================================
Files 642 644 +2
Lines 28358 28474 +116
Branches 6447 6461 +14
=============================================
+ Hits 20071 20148 +77
- Misses 8287 8326 +39 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should truncate the application name we use to pre-populate the host on step two of the application creation so it's within the host char limit.
@KlapTrap I thought about that, but as the change would not be visible the user would end up with an outcome they weren't expecting. The other alternative would be to fire the validation on change rather than defocus |
I'm happy with either. |
- template form failed to do this properly, converted to reactive - there's some odd code in here, it also fails when creating tcp routes (unrelated) - this whole step should be replaced with the create route stepper when merged into deploy
@KlapTrap Cool, I've made it so the form shows validation errors on enter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM