Max length for naming #275
Conversation
0del
force-pushed
the
max-length-temporary-name
branch
from
34db282
to
dfd9955
Compare
Co-authored-by: Andrew Farries <andyrb@gmail.com>
Co-authored-by: Andrew Farries <andyrb@gmail.com>
Co-authored-by: Andrew Farries <andyrb@gmail.com>
Signed-off-by: Alexis Rico <sferadev@gmail.com>
Signed-off-by: Alexis Rico <sferadev@gmail.com>
|
@andrew-farries Updated with main and applied the changes to make CI checks pass |
Signed-off-by: Alexis Rico <sferadev@gmail.com>
| } | ||
|
|
||
| func (e InvalidNameLengthError) Error() string { | ||
| return fmt.Sprintf("max length of %q is %d", e.Name, e.Max) |
There was a problem hiding this comment.
| return fmt.Sprintf("max length of %q is %d", e.Name, e.Max) | |
| return fmt.Sprintf("length of %q (%d) exceeds maximum length of %d", e.Name, len(e.Name), e.Max) |
| func validateName(name string) error { | ||
| if len(name) > MaxNameLength { | ||
| return InvalidNameLengthError{Name: name, Max: MaxNameLength} | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func ValidateName(name string) error { | ||
| return validateName(name) | ||
| } |
There was a problem hiding this comment.
Get rid of validateName and use the exported ValidateName everywhere.
|
|
||
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := validateName(o.Column); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
I don't think it makes sense to check the length of the column name on the operation here as it must already exist in the database. The check can be removed.
| if err := migrations.ValidateName(versionSchema); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
This is too late to check the length of the new version schema - at this point a potentially lengthy migration start operation has already taken place so it's a poor user experience for the migration to fail with a validation check at this stage. I'd remove the check here and if we need it we can add a check earlier in the migration start process as a separate PR.
|
Bump @0del 👋 This PR is almost ready to go; it just needs some removals. |
TODO list:
fixes: #64