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

Expose firestore query errors #26

Merged
merged 1 commit into from
Aug 15, 2018
Merged

Expose firestore query errors #26

merged 1 commit into from
Aug 15, 2018

Conversation

aaaronMF
Copy link
Contributor

Closes #12.

The code from @sampl was slightly modifed in order for existing tests to pass, and tests were added to ensure that the FirestoreCollection and FirestoreDocument components react accordingly to firestore query errors.

@aaaronMF
Copy link
Contributor Author

I noticed that tests are failing in the Travis CI build, but they are passing locally for me. Would I be able to get assistance in resolving this?

@sampl
Copy link
Contributor

sampl commented Jul 17, 2018

@aaaronMF thank you so much for jumping on this!

@green-arrow
Copy link
Owner

@aaaronMF - I pulled down your branch and am seeing the test failures that are in TravisCI locally. When you run yarn test, you will need to make sure all the tests are running by pressing a. Let me know if that works for you!

Also, can we remove the package-lock.json file from this PR?

…n and FirestoreDocument when errors occur; add testing for firestore query error handling
@codecov
Copy link

codecov bot commented Jul 18, 2018

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #26   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          78     80    +2     
  Branches       18     18           
=====================================
+ Hits           78     80    +2
Impacted Files Coverage Δ
src/FirestoreDocument.js 100% <100%> (ø) ⬆️
src/FirestoreCollection.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b90f528...ab71aa6. Read the comment docs.

@aaaronMF
Copy link
Contributor Author

Tests are passing now. @warbrett pointed out I should run yarn install instead of using npm to ensure the correct package versions are being used.

@sampl
Copy link
Contributor

sampl commented Aug 2, 2018

What do you think @green-arrow?

@green-arrow green-arrow merged commit d78f4b3 into green-arrow:master Aug 15, 2018
@green-arrow
Copy link
Owner

@aaaronMF @sampl - Sorry for the delay on this. I was on vacation for a couple weeks and been trying to catch back up at work.

Thanks for this PR!

@sampl
Copy link
Contributor

sampl commented Aug 15, 2018

Wooohoooo!! Thanks @green-arrow 😃

🙏🎇🚀🌟

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.

None yet

3 participants