[asterisk-dev] [Code Review] Allow configuration of minsecs and nextaftercmd per mailbox

[Jeff Peeler]

> On 2010-03-15 18:13:48, Tilghman Lesher wrote:
> > /trunk/apps/app_voicemail.c, line 1060
> > <https://reviewboard.asterisk.org/r/555/diff/1/?file=8581#file8581line1060>
> >
> >     Do we really need the compatibility option 'minmessage' for this, given that for the mailbox, it'll always be named 'minsecs'?

I guess not, removed.


> On 2010-03-15 18:13:48, Tilghman Lesher wrote:
> > /trunk/apps/app_voicemail.c, lines 10265-10272
> > <https://reviewboard.asterisk.org/r/555/diff/1/?file=8581#file8581line10265>
> >
> >     Given that these flags are copied from the global space for defaults, wouldn't it make sense to test both positive AND negative options, to ensure that the option is being parsed correctly for the mailbox and isn't just an inherited value?

I specifically did not call populate_defaults for this reason.


> On 2010-03-15 18:13:48, Tilghman Lesher wrote:
> > /trunk/apps/app_voicemail.c, lines 1061-1064
> > <https://reviewboard.asterisk.org/r/555/diff/1/?file=8581#file8581line1061>
> >
> >     I think it'd be preferable to use sscanf, since 0 is a perfectly legitimate minimum number of seconds for this option.
> >     
> >     And why copy from vmmaxsecs?  Shouldn't this be copied from vmminsecs?

Fixed to not reference max anymore. Sscanf preference observed.


- Jeff


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


On 2010-03-15 17:39:17, Jeff Peeler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/555/
> -----------------------------------------------------------
> 
> (Updated 2010-03-15 17:39:17)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Make this work in voicemail.conf:
> 
> [default]
> 1234 => 4242,Example Mailbox,imapuser at localhost,,minsecs=5|nextaftercmd=yes
> 
> 
> This addresses bug 16864.
>     https://issues.asterisk.org/view.php?id=16864
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_voicemail.c 252626 
> 
> Diff: https://reviewboard.asterisk.org/r/555/diff
> 
> 
> Testing
> -------
> 
> Tested both options to ensure they were respected and have also created a unit test that checks to see if the vm_user struct is populated correctly (I guess it'll protect against accidental deletions or typos).
> 
> 
> Thanks,
> 
> Jeff
> 
>