[asterisk-dev] [Code Review] 3313: Testsuite: Convert YAML Configuration Into Call File and Execute

[Scott Griepentrog]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140401/c2f099ab/attachment.html>