-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add DNS RFC 1035 label format validator #833
Conversation
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.
This looks fine to me other than a few small things.
@deankarn any objections to merging once review items are addressed?
{"abc-", "dns_rfc1035_label", false}, | ||
{"abc-123", "dns_rfc1035_label", true}, | ||
{"ABC", "dns_rfc1035_label", false}, | ||
{"ABC-123", "dns_rfc1035_label", false}, |
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.
Given that input may come from someone who has no idea what they're doing, what about some more obtuse edge cases like:
"abc-abc"
"ABC-ABC"
"123-abc"
""
?
validator_test.go
Outdated
validate := New() | ||
|
||
for i, test := range tests { | ||
|
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.
nit: remove this blank line? Blank lines are good to denote section breaks for improved readability, but here IMO it just makes it so less LOC fit on a single screen, which actually reduces readability. So just maybe be not quite so quick to insert one.
baked_in.go
Outdated
// isDnsRFC1035LabelFormat is the validation function for validating if the current field's value is a valid dns RFC 1035 label, defined in RFC 1035 | ||
func isDnsRFC1035LabelFormat(fl FieldLevel) bool { | ||
val := fl.Field().String() | ||
|
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.
nit: remove blank line?
baked_in.go
Outdated
@@ -2407,3 +2408,10 @@ func isIsoBicFormat(fl FieldLevel) bool { | |||
|
|||
return bicRegex.MatchString(bicString) | |||
} | |||
|
|||
// isDnsRFC1035LabelFormat is the validation function for validating if the current field's value is a valid dns RFC 1035 label, defined in RFC 1035 |
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.
Break this comment into mulitple lines and end with a period?
No need for it to keep going and going to the right...
@jeffwidman |
Enhances
Make sure that you've checked the boxes below before you submit PR:
Change details:
@go-playground/validator-maintainers