Ashley Sanders has posted comments on this change. Change subject: sip_attended_transfer now supports pre-12 Asterisk versions. ......................................................................
Patch Set 2: (4 comments) A vast improvement over the previous approach. I think this version is much easier for the human eye to follow the logic and glean intent, without having to use tools or execute the code directly. The only issues that I have raised have been minor issues for clarity of PEP8 compliance. You are very close, like inches and goal. Awesome work =) https://gerrit.asterisk.org/#/c/29/2/tests/channels/SIP/sip_attended_transfer/attended_transfer.py File tests/channels/SIP/sip_attended_transfer/attended_transfer.py: I think you should run pylint on this - you have a couple of places where the continuation indentation is not correct and you are missing docstrings for the class methods. Line 141: self.chans = [] I think you should go ahead and spell this out since it is on the class level. Line 142: self.final_bridge = 0 Being that this variable denotes how many times we received the AMI VarSet event for the expected channel/value combination and for the variable, BRIDGEPEER, I think a clearer name for this would be something like, "self.bridged_peers". Line 153: if numchans == 1: : self.controller.call_carol() : elif numchans == 2: : self.controller.transfer_call() > Since Asterisk 11 Bridge events are 'weird' (aka: confusing and prone to br I agree - while authoring the tests, we should do everything we can to help us later when debugging a broken asterisk. Future us will thank you greatly for better log messages :p -- To view, visit https://gerrit.asterisk.org/29 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d Gerrit-PatchSet: 2 Gerrit-Project: testsuite Gerrit-Branch: master Gerrit-Owner: Mark Michelson <[email protected]> Gerrit-Reviewer: Ashley Sanders <[email protected]> Gerrit-Reviewer: Mark Michelson <[email protected]> Gerrit-Reviewer: Matt Jordan <[email protected]> Gerrit-HasComments: Yes -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
