Skip to content

Skip undefined properties #353

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Skip undefined properties #353

wants to merge 4 commits into from

Conversation

thomas-ama
Copy link

@thomas-ama thomas-ama commented May 10, 2019 •
edited
Loading

PR for issue #308

@thomas-ama
Copy link
Author

I don't understand why tests are failing on Node.js stable but not on LTS. Any ideas?

@henrikra
Copy link
Contributor

henrikra commented Jun 5, 2019

This is something we really need also! Have you pinged any maintainers so we get this in?

@henrikra henrikra mentioned this pull request Jun 5, 2019
Closed
@thomas-ama
Copy link
Author

thomas-ama commented Jun 5, 2019 •
edited
Loading

@NoNameProvided @19majkel94 Have you had a chance to look at this?

henrikra
henrikra reviewed Jun 5, 2019
View reviewed changes
Copy link
Contributor

@henrikra henrikra left a comment

Choose a reason for hiding this comment

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

Looks good!

@henrikra
Copy link
Contributor

henrikra commented Jun 5, 2019

Also if you remove this line https://github.com/typestack/class-validator/blob/master/.travis.yml#L3 then the test will pass becase stable is getting node 12 which is not actually stable :P If you google the error on travis logs you will get lot of cases like this https://www.google.com/search?q=ReferenceError%3A+primordials+is+not+defined&oq=ReferenceError%3A+primordials+is+not+defined&aqs=chrome..69i57j69i58j69i60.220j0j7&sourceid=chrome&ie=UTF-8

@thomas-ama
Copy link
Author

Thanks @henrikra, let's say tests are okay then.

@henrikra
Copy link
Contributor

This looks golden to me ⭐️

@greg-hoarau
Copy link

greg-hoarau commented Jun 19, 2019 •
edited
Loading

Hello, do you have any idea when this flag will be available ? NEED 😄

And thanks to @thomas-ama for this PR !

@henrikra
Copy link
Contributor

Lets merge this!!

@vlapo
Copy link
Contributor

vlapo commented Jul 21, 2019

@thomas-ama could you please rebase and add also skipNullProperties option? We should deprecate skipMissingProperties in future and use skipNullProperties and skipUndefinedProperties instead.

@vlapo vlapo added this to the 0.10.1 milestone Jul 26, 2019
@ruscon ruscon mentioned this pull request Sep 6, 2019
Merged
@vlapo
Copy link
Contributor

vlapo commented Sep 6, 2019

Close in favore of #414. @thomas-ama thank you for your effort.

@vlapo vlapo closed this Sep 6, 2019
@github-actions
Copy link

github-actions bot commented Aug 4, 2020

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@henrikra henrikra henrikra left review comments

@iveselin iveselin iveselin approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
0.10.1
Development

Successfully merging this pull request may close these issues.
5 participants
@thomas-ama @henrikra @greg-hoarau @vlapo @iveselin