[asterisk-dev] [Code Review] 3991: musiconhold: Add sort=randstart, and deprecate old stuff.

[wdoekes]

> On Sept. 12, 2014, 4 p.m., one47 wrote:
> > Personal preference, but is it appropriate to include the "sort" value in the output of "moh show classes" while we're tidying things up?
> > 
> > Just a thought.

Certainly appropriate. But since I won't be seeing any of it because I use realtime moh, it's not on the top of my list.

If you like the feature, feel free to code it up. And you get to defend the extra room it takes in the output ;-)

Thanks for the review though (and you, Mark).


- wdoekes


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


On Sept. 12, 2014, 12:22 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3991/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2014, 12:22 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This changeset:
> 
> - adds sort=randstart (next to sort=, sort=random, sort=alpha)
> 
> - combines duplicate moh option parsing code into a single function
> 
> - adds deprecationwarnings for application=r to sort randomly
> 
> - adds deprecationwarnings for random=yes to sort randomly
> 
> - removes invisible code that was supposed to stay until 1.8 
>   ("These names were deprecated in 1.4 and should not be used until
>     after the next major release")
> 
> 
> The sort=randstart works like sort=alpha, except we start at a random
> position.
> 
> That means that playlist order will be respected while you don't need
> to listen to the same song every time you dial in.
> 
> 
> I considered also adding a sort=shuffle which does a shuffle where
> sort=alpha now does the qsort. But that would probably only be useful
> for those few people that use uncached (realtime) classes.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_musiconhold.c 422983 
>   /trunk/configs/samples/musiconhold.conf.sample 422983 
> 
> Diff: https://reviewboard.asterisk.org/r/3991/diff/
> 
> 
> Testing
> -------
> 
> Tested that sort=, sort=alpha, sort=random, sort=randstart all work as
> promised.
> 
> (Testing was done on an asterisk-10 branch with uncached realtime moh.
> But the code in trunk is exactly the same, apart from the
> ast_format_cache_get to ast_format_cache_get change.)
> 
> 
> Checked that the new option 'randstart' fits within the current alembic
> DB schema. It does:
> 
>     op.create_table(
>         'musiconhold',
>         sa.Column('name', sa.String(80), primary_key=True, nullable=False),
> ...
>         sa.Column('sort', sa.String(10)),
>         sa.Column('format', sa.String(10)),
>         sa.Column('stamp', sa.DateTime())
>     )
> 
> 
> Thanks,
> 
> wdoekes
> 
>

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