-
-
Notifications
You must be signed in to change notification settings - Fork 621
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
Fix imports of FCM to keep it as an optional dependency #706
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #706 +/- ##
==========================================
- Coverage 70.33% 70.28% -0.06%
==========================================
Files 26 26
Lines 1099 1097 -2
Branches 249 249
==========================================
- Hits 773 771 -2
Misses 288 288
Partials 38 38 ☔ View full report in Codecov by Sentry. |
push_notifications/models.py
Outdated
@@ -1,9 +1,7 @@ | |||
from django.db import models | |||
from django.utils.translation import gettext_lazy as _ | |||
from firebase_admin import messaging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has instead been fixed with #707
Thank you for making this MR. The patch was instead resolving two issues, making #707 resolved the |
61d9f70
to
c54230f
Compare
I have rebased over #707, now this PR makes a single import statement instead of two to handle FCM in models.py. |
I would make one more quick pass, and merge into the Once again, thank you for pulling in this patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
As pointed out in this comment #702 (comment) importing
firebase_admin
orgcm
at top level inmodels.py
turns thefirebase_admin
package as a mandatory requirements while it should not.To address this I have moved the imports inside the appropriated methods. This allow the codebase to run even without the firebase package (which is optional).