Skip to content

Add ability to ignore validation on Parse.Object constructor/set() #74

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

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

TylerBrock
Copy link
Contributor

This is an attempt to resolve the first blocker in #73.

Not sure if the Flow typing is correct but I'm sure you'll let me know if it is not.

@codecov-io
Copy link

Current coverage is 85.30%

Merging #74 into master will decrease coverage by -0.02% as of a51c23b

@@            master    #74   diff @@
=====================================
  Files           37     37       
  Stmts         2793   2796     +3
  Branches         0      0       
  Methods          0      0       
=====================================
+ Hit           2383   2385     +2
  Partial          0      0       
- Missed         410    411     +1

Review entire Coverage Diff as of a51c23b

Powered by Codecov. Updated on successful CI builds.

@TylerBrock
Copy link
Contributor Author

The coverage tool doesn't seem to understand the constructor arguments being split into multiple lines. Cest la vies, eh?

@TylerBrock TylerBrock force-pushed the add-ignore-validation branch from fd8bcd2 to 02a339c Compare October 29, 2015 21:34
@@ -627,7 +631,7 @@ export default class ParseObject {

// Validate changes
var validation = this.validate(newValues);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thought: it might make more sense to wrap this entire var validation = ...; if (...) {} block in an
if (!options.ignoreValidation) statement. That way, we don't needlessly run the validation logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for sure. I'll update the request.

@TylerBrock TylerBrock force-pushed the add-ignore-validation branch from 02a339c to f823344 Compare October 29, 2015 21:39
@TylerBrock
Copy link
Contributor Author

I'm also not thrilled with the API. I think a {deferValidation: true} on the constructor that just defers validation for all sets until persistence time might be better. Would love to hear your thoughts.

@TylerBrock TylerBrock force-pushed the add-ignore-validation branch 3 times, most recently from f26bd9a to 8abbc52 Compare October 29, 2015 23:22
@TylerBrock TylerBrock force-pushed the add-ignore-validation branch from 8abbc52 to e7e42a5 Compare October 30, 2015 18:43
@TylerBrock
Copy link
Contributor Author

Bump, lets get this rocking and rolling. I'd like to update our codebase without fear that this isn't going into the SDK. If there are issues or concerns lets discuss.

@andrewimm
Copy link
Contributor

I like it. I think every entry point is covered (there are a lot of constructors due to supporting the old .extends()). Tests seem good to me.

andrewimm added a commit that referenced this pull request Nov 3, 2015
Add ability to ignore validation on Parse.Object constructor/set()
@andrewimm andrewimm merged commit 4499860 into parse-community:master Nov 3, 2015
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