[SR-Users] Freezing development for Kamailio v4.1.0

[Daniel-Constantin Mierla]

On 10/2/13 8:25 AM, Timo Teras wrote:
> On Mon, 30 Sep 2013 14:46:48 +0200
> "Olle E. Johansson" <oej at edvina.net> wrote:
>
>> 30 sep 2013 kl. 14:04 skrev Daniel-Constantin Mierla
>> <miconda at 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 <miconda at 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.

 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(...)

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|...]/.

Cheers,
Daniel

-- 
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28; Miami, Nov 18-20, 2013
   - more details about Kamailio trainings at http://www.asipto.com -