Code review comment for lp:~rhuddie/ubuntu-push/push-autopilot-tests

Revision history for this message
Leo Arias (elopio) wrote :

53 +5) autopilot3 list push-notifications
54 +6) autopilot3 run push-notifications

You have a typo there ^. It should be push_notifications.

166 +class PushNotificationMessage:
187 +class NotificationData:

I would put these classes in a push_notifications/data.py module.

210 + if dbus_info != None:
215 + elif copy_obj != None:

You should make checks like these with 'is not' instead of !=.
That's from pep8: "Comparisons to singletons like None should always be done with is or is not, never the equality operators."

203 + def __init__(self, dbus_info=None, copy_obj=None):

I find this to be a confusing constructor. I think it would be clearer to make two @classmethods, like
def from_dbus_info(dbus_info=None)
def copy(original_object=None)

Actually, I'm not sure you would need that copy. You could use the copy module.

review: Needs Fixing

« Back to merge proposal