-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: add image validation #982
feat: add image validation #982
Conversation
46de0f3
to
bc7329e
Compare
@hjwk can you help review this please? |
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.
Sure, can do a quick review but I am not an owner...
Don't have much to say, but please check code alignment (I've highlighted one example in the review)
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.
A few changes and I think this PR would be good to go.
baked_in.go
Outdated
_, err = file.Seek(0, io.SeekStart) | ||
if err != nil { | ||
panic(fmt.Sprintf("Could not reset seek start for file. Error: %s", err)) | ||
} |
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.
I'm not sure why the seek is required? should be removed from what I can tell.
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.
While reading data on the mime type of the file, the offset for the reader is moved away from the beginning of the file and this may cause issues for other parts of the program that need to read from that file. Using seek ensures that after the mime type is read, the offset of the reader is returned to the beginning of the file so that there will be no issues for other parts of the program that will need to read from that file.
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.
Right I understand what Seek
does and why one would need to use it but in this case the file handle is closed when this function ends because of the defer file.Close()
and nothing else tries to read using that handle and so there is no need to Seek from what I can tell.
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.
Right, got you.
Sync With Upstream
…feat-add-image-validation
Let's ship this please 👍 |
Sync with upstream
…feat-add-image-validation
@@ -1321,6 +1321,11 @@ func RegisterDefaultTranslations(v *validator.Validate, trans ut.Translator) (er | |||
translation: "{0} deve ser um valor booleano válido", | |||
override: false, | |||
}, | |||
{ | |||
tag: "image", | |||
translation: "{0} deve ser uma imagen válido", |
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.
The correct translation is
"{0} deve ser uma imagem válida"
Enhances
Make sure that you've checked the boxes below before you submit PR:
☑️ Tests exist or have been written that cover this particular change.
This PR does the following:
README.md
document to include the added image validationMotivation:
@go-playground/validator-maintainers