[asterisk-dev] [Code Review]: Fix asttest's Asterisk version extraction

Matt Jordan reviewboard at asterisk.org
Sat Jan 28 21:35:02 CST 2012



> On Jan. 28, 2012, 9:03 p.m., Russell Bryant wrote:
> > Just some minor suggestions

Thanks for a Saturday night review!


> On Jan. 28, 2012, 9:03 p.m., Russell Bryant wrote:
> > /asterisk/trunk/asttest/lib/lua/astlib.lua, line 44
> > <https://reviewboard.asterisk.org/r/1701/diff/1/?file=23798#file23798line44>
> >
> >     It looks like spaces after the commas here would be more consistent with other code

Fixed


> On Jan. 28, 2012, 9:03 p.m., Russell Bryant wrote:
> > /asterisk/trunk/lib/python/asterisk/version.py, line 79
> > <https://reviewboard.asterisk.org/r/1701/diff/1/?file=23799#file23799line79>
> >
> >     Walter also mentioned that you could mark this as a @classmethod since it doesn't use any member variables.

Whoops, missed that.  Thanks


> On Jan. 28, 2012, 9:03 p.m., Russell Bryant wrote:
> > /asterisk/trunk/lib/python/asterisk/version.py, line 83
> > <https://reviewboard.asterisk.org/r/1701/diff/1/?file=23799#file23799line83>
> >
> >     Parens aren't necessary here.

And no they're not.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1701/#review5350
-----------------------------------------------------------


On Jan. 28, 2012, 3:26 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1701/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2012, 3:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch changes how the lua-based tests determine the Asterisk version.  Previously, like the version.py library, the astlib.lua library was extracting this from the version.h header file.  Now it also spawns an Asterisk process with the '-V' command line switch and reads the version from the output.
> 
> This should resolve the failing tests in trunk:
> * rfc2833_dtmf_detect
> * manager/action-events-response
> * queues/position_priority_maxlen
> * cdr/blind-transfer-accountcode
> 
> This patch also addresses Russell and Walter's last comments on version.py in https://reviewboard.asterisk.org/r/1695/.  Specifically, it re-raises caught OSError exceptions after logging them, and places the setting of the static variable in a private method.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/asttest/lib/lua/astlib.lua 3021 
>   /asterisk/trunk/lib/python/asterisk/version.py 3021 
> 
> Diff: https://reviewboard.asterisk.org/r/1701/diff
> 
> 
> Testing
> -------
> 
> Ran on local development machine; all currently failing trunk tests passed.
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120129/e08f2386/attachment.htm>


More information about the asterisk-dev mailing list