Skip to content
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 null check to verifyIfObjectsMatch #58

Merged
merged 5 commits into from
Jul 28, 2020
Merged

Add null check to verifyIfObjectsMatch #58

merged 5 commits into from
Jul 28, 2020

Conversation

tobloef
Copy link
Contributor

@tobloef tobloef commented Jul 19, 2020

This was causing some trouble for me. To give some context, I encountered this in my Cypress tests using cypress-react-selector, which is dependent on resq. I'm not sure exactly what is causing the argument to be null, but this seemed like the easiest fix.

@abhinaba-ghosh
Copy link

hi @baruchvlz , kindly look into PR. It will help the cypress community in a way.

Copy link
Owner

@baruchvlz baruchvlz left a comment

Choose a reason for hiding this comment

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

Hey @tobloef, thanks for looking into this. For me, this is an odd bug since the verify has a default value in the function. As long as the test pass I'm ok with the change.

src/utils.js Outdated Show resolved Hide resolved
@tobloef
Copy link
Contributor Author

tobloef commented Jul 23, 2020

Thanks. As far as I know, the default value is only used if the passed argument is undefined, not null, as odd as that seems.

@tobloef
Copy link
Contributor Author

tobloef commented Jul 25, 2020

Tried to update it based on your feedback. Let me know what you think.

@tobloef tobloef requested a review from baruchvlz July 25, 2020 18:40
Copy link
Owner

@baruchvlz baruchvlz left a comment

Choose a reason for hiding this comment

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

Please remove semi-colons.

@tobloef tobloef requested a review from baruchvlz July 28, 2020 08:14
Copy link
Owner

@baruchvlz baruchvlz left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this. I will do a release later today.

@baruchvlz baruchvlz merged commit 0dbb4ba into baruchvlz:master Jul 28, 2020
@abhinaba-ghosh
Copy link

@baruchvlz do we have a new release yet? thanks.

@baruchvlz
Copy link
Owner

@abhinaba-ghosh @tobloef I'm really sorry y'all, completely slipped my mind to publish the package. v1.8.0 should be available now.

@abhinaba-ghosh
Copy link

Thanks a lot. 😇

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.

3 participants