sucks
sucks copied to clipboard
wpietri
Fix status #57
Initial status responses were missing the 'td' attrib so line 462 was causing it to just return instead of calling the handler. Added conditions to check for attribs and tags to assign correct event. Still handles original formatted responses with 'td' present.
I appreciate the patch! Could you please also add some tests that a) will keep people from breaking this in the future, and b) convey the purpose of the code? See the tests folder for examples. Thanks!
I'll give it a try when I get a chance - I have never worked with the testing modules like that so I will try to learn what you're doing there and add some. It looks like I need to add a test with the type of xml my changes are supposed to work with and ensure that it detects it correctly.
~jason
On Sun, Dec 23, 2018 at 10:16 AM William Pietri [email protected] wrote:
I appreciate the patch! Could you please also add some tests that a) will keep people from breaking this in the future, and b) convey the purpose of the code? See the tests folder for examples. Thanks!
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wpietri/sucks/pull/61#issuecomment-449646774, or mute the thread https://github.com/notifications/unsubscribe-auth/AGUvkVAVhGV2_9QiwxR43vxa2U8Wi3Khks5u76xegaJpZM4ZfskW .
Added a few tests, hopefully that helps and also explains what the change is fixing.