re: Gcm::Delivery reorganization:
the primary goal was to make code paths easier to follow in the case of a complete success, complete network fail/retry, and partial success/failure. even within these there are overlapping concerns, so the processing of the result data is unified into a new data structure (Gcm::Results) optimized for the different processing requirements - this happens first. from there the conditions leading to different side-effects are clearer. for example, there is now only one place where a DeliveryError is raised after a 200 response and one place where most of the error messages are generated for that.
the only functional change - apart from the two new reflections - is that in the case where all failures were invalid ids it used to mark the delivery as failed but not raise a DeliveryError - now it raises the error. also these error results are included in the error messages like ay others. the original behavior seemed incongruous and resulted in additional complexity. i can see a case for doing this so that one is effectively treating this class of errors (INVALID_REGISTRATION_ID_STATES) one way and others another way, but the UNAVAILABLE_STATES class of errors should then also be treated separately...
ileitch
commented
10 years ago
re: Gcm::Delivery reorganization: the primary goal was to make code paths easier to follow in the case of a complete success, complete network fail/retry, and partial success/failure. even within these there are overlapping concerns, so the processing of the result data is unified into a new data structure (Gcm::Results) optimized for the different processing requirements - this happens first. from there the conditions leading to different side-effects are clearer. for example, there is now only one place where a DeliveryError is raised after a 200 response and one place where most of the error messages are generated for that. the only functional change - apart from the two new reflections - is that in the case where all failures were invalid ids it used to mark the delivery as failed but not raise a DeliveryError - now it raises the error. also these error results are included in the error messages like ay others. the original behavior seemed incongruous and resulted in additional complexity. i can see a case for doing this so that one is effectively treating this class of errors (INVALID_REGISTRATION_ID_STATES) one way and others another way, but the UNAVAILABLE_STATES class of errors should then also be treated separately...