-
Notifications
You must be signed in to change notification settings - Fork 30
bug: Remove discussion comment and useless bool return #1027
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1027 +/- ##
==========================================
- Coverage 99.76% 99.76% -0.01%
==========================================
Files 55 55
Lines 8802 8801 -1
==========================================
- Hits 8781 8780 -1
Misses 21 21
Continue to review full report at Codecov.
|
autopush/db.py
Outdated
@@ -436,7 +436,7 @@ def save_channels(self, uaid, channels): | |||
|
|||
@track_provisioned | |||
def store_message(self, notification): | |||
# type: (WebPushNotification) -> bool | |||
# type: (WebPushNotification) |
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.
mypy wants -> None here (don't ask me why because I do this all the time)
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.
Oh, huh, and pycharm noted that. Need to make it a more important warning.
@@ -448,7 +448,6 @@ def store_message(self, notification): | |||
updateid=notification.update_id | |||
) | |||
self.table.put_item(data=item, overwrite=True) | |||
return True |
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.
now we should stop checking for this result in route_notification too (and remove the silly "result is False" line)
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.
AH! I was looking at the wrong .save_notification
and wondering what you meant.
And now I'm disturbed that removing that condition and the returned error didn't cause any test to fail or coverage line to be lost. (sigh)
Closes #1026