----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3313/#review11471 -----------------------------------------------------------
./asterisk/trunk/lib/python/asterisk/pluggable_modules.py <https://reviewboard.asterisk.org/r/3313/#comment21172> Pep8 wants 2 blank lines prior to the start of a class definition. ./asterisk/trunk/lib/python/asterisk/pluggable_modules.py <https://reviewboard.asterisk.org/r/3313/#comment21170> Nitpic: Because you've got an open (, the \ at the end of the following lines is not necessary. Also, these lines are not formatted to avoid pep8 warnings. ./asterisk/trunk/lib/python/asterisk/pluggable_modules.py <https://reviewboard.asterisk.org/r/3313/#comment21173> The use of with causes the lifetime of the outfile variable to be limited to the block of code indented below it. When the lifetime of outfile expires, it is closed by python -- thus your attempt to outfile.close() is not necessary. ./asterisk/trunk/lib/python/asterisk/pluggable_modules.py <https://reviewboard.asterisk.org/r/3313/#comment21171> Here you don't have an ( pending on the first line, so that \ is required, but the following ones aren't. Also pep8 format warnings. ./asterisk/trunk/lib/python/asterisk/pluggable_modules.py <https://reviewboard.asterisk.org/r/3313/#comment21174> Be aware that the attempt to update the time on the file has a possibility of failing, if Asterisk has already responded to the file being moved into outgoing and deleted it. I wouldn't remove the utime() however, as it insures that Asterisk will run it. Since you're not checking the return value, this is correct. However, I would recommend that you check the return value on the move and log any failures. - Scott Griepentrog On April 1, 2014, 2:57 p.m., Scott Emidy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3313/ > ----------------------------------------------------------- > > (Updated April 1, 2014, 2:57 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23217 > https://issues.asterisk.org/jira/browse/ASTERISK-23217 > > > Repository: testsuite > > > Description > ------- > > This test uses the Pluggable Module Framework in order to convert a YAML > configuration into an executable Call File by putting it into the correct > directory, which is the 'astspooldir' directory. > > > Diffs > ----- > > ./asterisk/trunk/tests/pbx/tests.yaml 4726 > ./asterisk/trunk/tests/pbx/create_call_files/test-config.yaml PRE-CREATION > ./asterisk/trunk/tests/pbx/create_call_files/configs/ast1/extensions.conf > PRE-CREATION > ./asterisk/trunk/sample-yaml/callfiles-config.yaml.sample PRE-CREATION > ./asterisk/trunk/lib/python/asterisk/pluggable_modules.py 4726 > > Diff: https://reviewboard.asterisk.org/r/3313/diff/ > > > Testing > ------- > > > Thanks, > > Scott Emidy > >
-- _____________________________________________________________________ -- 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
