Skip to content

Conversation

@msokoloff1
Copy link
Contributor

@msokoloff1 msokoloff1 commented Mar 4, 2021

  • Client side validation for MAL

Copy link
Contributor

@florijanstamenkovic florijanstamenkovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial fast review looking mostly at code organization and no logic.

Also, please name PR something more descriptive. Just "validate" can mean many things.

@msokoloff1 msokoloff1 changed the title Ms/validation MAL Validation Mar 8, 2021
Copy link
Contributor

@florijanstamenkovic florijanstamenkovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still some things to fix up, please address.

Comment on lines 109 to 111
class ValidationError(LabelboxError):
"""Raised when user input is invalid for MAL imports."""
...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a more descriptive name, e.g. AnnotationImportValidationError?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is kind of long. How is MALValidationError?

@msokoloff1 msokoloff1 merged commit 829884a into develop Mar 10, 2021
@msokoloff1 msokoloff1 deleted the ms/validation branch March 10, 2021 19:33
msokoloff1 added a commit that referenced this pull request Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants