On Wed, 02 Oct 2013 15:20:33 +0200 Daniel-Constantin Mierla <mico...@gmail.com> wrote:
> > On 10/2/13 8:25 AM, Timo Teras wrote: > > On Mon, 30 Sep 2013 14:46:48 +0200 > > "Olle E. Johansson" <o...@edvina.net> wrote: > > > >> 30 sep 2013 kl. 14:04 skrev Daniel-Constantin Mierla > >> <mico...@gmail.com>: > >> > >>> Hello, > >>> > >>> On 9/30/13 12:08 PM, Timo Teras wrote: > >>>> On Mon, 30 Sep 2013 11:48:10 +0200 > >>>> Daniel-Constantin Mierla <mico...@gmail.com> wrote: > >>>> > >>>>> a short reminder that we are one week before feature freeze for > >>>>> next major release. > >>>> I would like to incorporate the module from: > >>>> https://github.com/rdboisvert/mohqueue > >>>> > >>>> Could you review it? > >>>> > >>>> I am willing to be maintainer for it if needed. > >>> interesting feature - no need for special review because it is a > >>> stand alone module. You are already registered developer, so you > >>> can review yourself a bit if you didn't do (use) it already - the > >>> review of new modules from new developers is to check quickly if > >>> they use internal memory managers, DB API and/or spot if something > >>> else from existing code can be reused. > >> Agree, this is a really cool feature. I can think of a ton of RPC > >> commands as well as stats/counters I would like to see. We're > >> looking forward to this! > > The author asked me to do the initial merge. He might be willing to > > take over the maintainer ship in few weeks, and we can come back > > to this discussion after a bit. > > > > I created 'tteras/mohqueue' branch and pushed the module there > > as-is - with LICENSE removed as being redundant now that it is part > > of the main tree. > > > > If possible please take a look at it and post any comments to fix > > before merge to master, which I plan to do tomorrow. > > > > Robert, I also saw that you had created new branch 'noRTPstop' with > > three additional commits - please let me know if those are to be > > included too. Let me know also if there's any additional changes > > you'd like to have in. > The code indentation style is hard to follow for me, because the > blocks are not easy visible (for what I used to, probably). I would > prefer a style using tabs, with blocks indenting the inner acctions > (e.g., like in the core and most of the modules, that should be > pretty much Allman or K&R). Anyhow, it is not a must as long as > developer of that code keeps maintaining the code and doesn't need > help from others. I had the same comment. I'll just run it through indent. > From a quick look at main file in the module: > - fixup_str() can be replaced with fixup_spve_spve(...) - it is a > function that allows static and dynamic string parameters > - fixup_count() is then fixup_spve_null(...) Ok, good points. > The sql tables definition have to be made in xml and added to > lib/srdb1/schema/, then the sql script will be generated for all db > connectors via: > > make dbschema > > So modules/mohqueue/mohqueue.sql should not be part of the module -- > sql scripts will be part of utils/kamctl/[mysql|postgres|...]/. Oh right, forgot this. I also noted that the locking could be simplified, and use rwlock primites instead of rewriting it as spinlocks. -Timo _______________________________________________ SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list sr-users@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
