----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3061/#review10383 -----------------------------------------------------------
Richard asked me to take a look at this and limit my review to Part 1 from the description. In general, the change to use a struct instead of individual function pointers when registering voicemail functions is a good thing. However, in light of the context-related criticisms, the individual function pointers that expect a user and context should be altered to expect a single mailbox string instead. - Mark Michelson On Dec. 9, 2013, 9:51 p.m., rmudgett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3061/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2013, 9:51 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > This patch can be broken into four main components: > > 1) Voicemail callback registration/unregistration function improvements are > in the following files: > include/asterisk/doxyref.h, > include/asterisk/app.h, > main/app.c, and > apps/app_voicemail.c. > > * The voicemail registration/unregistration functions now take a struct of > callbacks instead of a lengthy parameter list of callbacks. > > * The voicemail registration/unregistration functions now prevent a competing > module from interfering with an already registered callback supplying module. > > 2) The core resource support for external MWI providers are in the following > files: > include/asterisk/res_mwi_external.h, > configs/sorcery.conf.sample, > res/res_mwi_external.c, and > res/res_mwi_external.exports.in. > > * The core external MWI resource provides for MWI message counts persistence > using sorcery. With sorcery, the user is able to configure which sorcery > wizzard backend to use if the default astdb is not desired. > > * The core external MWI resoruce provides some CLI commands to manually > manage the MWI counts if needed. > The new CLI commands are: > "mwi external delete all", > "mwi external delete like <regex>", > "mwi external delete mailbox <box[@[context]]>", > "mwi external list all", > "mwi external list like <regex>", > "mwi external show mailbox <box[@[context]]>", and > "mwi external update mailbox <box[@[context]]>". > > 3) The AMI component of external MWI is in: > res/res_mwi_external_ami.c > > * The external MWI AMI interface provides a thin wrapper around the core > external MWI resource. > The resource adds the following AMI actions: > MWIExternalGet, > MWIExternalDelete, and > MWIExternalUpdate. > > 4) A consistency fix to sorcery astdb regex record retrieval is in: > res/res_sorcery_astdb.c > > * The AstDB wizzard is inconsistent with the other sorcery wizzards with its > regex record selection. Any pattern that uses the '^' regex character to > anchor the pattern to the beginning of the key *must* be a simple prefix. > Any wildcard matching used with the '^' anchor will fail. As a result, a > regex of "^.*@context$" will not return anything even if there are mailboxes > in "context". > > > Diffs > ----- > > /trunk/res/res_sorcery_astdb.c 403574 > /trunk/res/res_mwi_external_ami.c PRE-CREATION > /trunk/res/res_mwi_external.exports.in PRE-CREATION > /trunk/res/res_mwi_external.c PRE-CREATION > /trunk/main/app.c 403574 > /trunk/include/asterisk/res_mwi_external.h PRE-CREATION > /trunk/include/asterisk/doxyref.h 403574 > /trunk/include/asterisk/app.h 403574 > /trunk/configs/sorcery.conf.sample 403574 > /trunk/apps/app_voicemail.c 403574 > > Diff: https://reviewboard.asterisk.org/r/3061/diff/ > > > Testing > ------- > > Used the CLI "database show" along with the new CLI commands and AMI actions > to test adding/updating, getting, and deleting external MWI counts. > > > Thanks, > > rmudgett > >
-- _____________________________________________________________________ -- 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
