[asterisk-dev] How to get peer review for patch to deprecated module

[Alex Villací­s Lasso]

El 11/04/15 a las 22:59, Matthew Jordan escribió:
> On Sat, Apr 11, 2015 at 4:31 PM, Alex Villacís Lasso
> <a_villacis at palosanto.com> wrote:
>> El 10/04/15 a las 14:16, Alex Villací­s Lasso escribió:
>>
>> El 08/04/15 a las 08:22, Vinicius Fontes escribió:
>>
>> Have you tried Asterisk 13? The bridging mechanism has been completely
>> rewritten on Asterisk 12, so there's no longer channel masquerading and
>> zombie channels. Might be worth a try.
>>
>> Sorry, this client is very hard to talk into stopping its operations long
>> enough to install changes, let alone a major Asterisk version change. I
>> already had trouble convincing him of the need to install a debugging
>> version with DEBUG_THREADS enabled.
>>
>> After reviewing a "core show locks" output, I am very sure I have hit an
>> Asterisk bug involving a deadlock in cdr_mysql . When doing a "core reload"
>> on the CLI, cdr_mysql calls ast_cdr_unregister() with its internal lock held
>> (mysql_lock). However, writing a CDR involves adquiring the cdr lock and
>> then the internal lock. Therefore, deadlock. None of the other cdr modules
>> does that.
>>
>> However, I know that cdr_mysql is currently deprecated for Asterisk 11. I
>> have a patch to fix the issue mentioned above (attached), but I want to know
>> how to get it reviewed, if only to see whether the fix is sane.
>>
> First, regardless of the module support state, if the module exists in
> Asterisk, it can be patched. The module support state only reflects
> the way in which support of a module is handled; if you write a patch
> for a module and would like to contribute it, that's always welcomed.
>
> Second, I suspect you are hitting this bug:
>
> https://issues.asterisk.org/jira/browse/ASTERISK-20347
>
> If you would like to have it re-opened, I'd be happy to do so.
>
> Third, patches via e-mail are typically not viewed (although your
> mileage may vary). Part of the CLA stipulates that, when submitting a
> patch to Asterisk, you are verifying that you are either the author of
> the patch, or have the permission of the authors to contribute the
> patch back to Asterisk. Over e-mail, it is difficult to ascertain if
> someone has (a) signed a CLA, and (b) wants to contribute a patch back
> to Asterisk under the CLA. As such, you'll need to submit the patch
> via one of the project mechanisms.
>
> Normally, that would mean attaching the patch to an issue in JIRA
> and/or submitting it to Review Board. Alas, this is a bit of an
> awkward weekend, as the Asterisk project is in the middle of moving
> from Subversion to Git. As a result, things are in a bit of a state of
> flux.
>
> I'd recommend doing the following:
>
> * Re-open ASTERISK-20347 and attach the patch to the issue after
> signing a CLA [1]. Regardless of the state of the source control for
> Asterisk, your patch will at least be tracked in the project. Again,
> I'll be happy to re-open the issue once you've confirmed you're
> willing to sign a CLA and/or have already done so.
I have already signed a license agreement for user a_villacis on JIRA. I have also uploaded the patch (against 11.17.0) on ASTERISK-20347. How do I proceed to get the bug reopened?
> * If you'd like, after signing a CLA, you can submit the patch for
> review on Gerrit [2]. Clone the 'asterisk' repo on Gerrit [3], apply
> the patch to 'master', and submit it using 'git review'. Draft
> instructions for using Gerrit can also be found on the wiki [4].
>
> If you'd like immediate feedback, I'd highly suggest posting the patch
> to Gerrit. Code reviews are a normal part of the Asterisk developer
> workflow, and we typically look for patches on tools that support
> that.
>
> [1] https://issues.asterisk.org/jira/secure/DigiumLicense.jspa
> [2] https://gerrit.asterisk.org/#/
> [3] https://gerrit.asterisk.org/#/admin/projects/asterisk
> [4] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
>