[asterisk-dev] [Code Review]: Bring the (missing) changes from Mantis ID 13495 in trunk.
KNK
reviewboard at asterisk.org
Thu Jul 19 08:32:26 CDT 2012
> On July 18, 2012, 5:05 p.m., rmudgett wrote:
> > You should not have added all the whitespace and curly brace cleanup changes to locations that you are not needing to change. They make an already large patch unnecessarily huge.
> >
> > Also tabs should be set to 4 not 8.
> >
> > I will continue to look at your patches.
Initially i have not touched whitespaces and braces, but after each change i got new red blocks nearby, so i just went ahead and fixed all surrounding places.
The location changes were made in order to make 1.0 and (ex)trunk compatible in order to keep the NEW_API branch to be able to receive new changes and not to loose it's functionality for the future. A similar patch against (ex)trunk will be provided in the issue.
I reconfigured my editor with tabs set to 4, but i am not sure, should i realign the code now (as i can see it is mostly in headers) or post another patch later with additional curly brace cleanups too, which will be much easier to review?
> On July 18, 2012, 5:05 p.m., rmudgett wrote:
> > branches/1.0/ss7linktest.c, line 326
> > <https://reviewboard.asterisk.org/r/1653/diff/14/?file=30260#file30260line326>
> >
> > Shouldn't this be:
> > ..., fd, -1, dpc)...
Good catch. Fixed
> On July 18, 2012, 5:05 p.m., rmudgett wrote:
> > branches/1.0/ss7test.c, line 242
> > <https://reviewboard.asterisk.org/r/1653/diff/14/?file=30261#file30261line242>
> >
> > Shouldn't this be:
> > ..., fds[1], -1, 1)...
Good catch. Fixed.
> On July 18, 2012, 5:05 p.m., rmudgett wrote:
> > branches/1.0/mtp3.c, line 395
> > <https://reviewboard.asterisk.org/r/1653/diff/14/?file=30256#file30256line395>
> >
> > Delete this line. The error has already been reported by ss7_next_empty_event().
Done
> On July 18, 2012, 5:05 p.m., rmudgett wrote:
> > branches/1.0/mtp3.c, line 508
> > <https://reviewboard.asterisk.org/r/1653/diff/14/?file=30256#file30256line508>
> >
> > Delete this line. The error has already been reported by ss7_next_empty_event().
Done
- KNK
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1653/#review6718
-----------------------------------------------------------
On July 13, 2012, 2:41 p.m., KNK wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1653/
> -----------------------------------------------------------
>
> (Updated July 13, 2012, 2:41 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> libss7 part of the changes.
> Added additional cause codes, Transmission Medium Requirement setting and connected line to CPG messages + code cleanup.
>
>
> This addresses bugs SS7-21, SS7-27, SS7-28, SS7-33, SS7-36, SS7-38, SS7-39, SS7-40, SS7-42, SS7-43, SS7-45, SS7-46, SS7-47, SS7-48, SS7-49, SS7-51, SS7-52, SS7-53, SS7-54, and SS7-7.
> https://issues.asterisk.org/jira/browse/SS7-21
> https://issues.asterisk.org/jira/browse/SS7-27
> https://issues.asterisk.org/jira/browse/SS7-28
> https://issues.asterisk.org/jira/browse/SS7-33
> https://issues.asterisk.org/jira/browse/SS7-36
> https://issues.asterisk.org/jira/browse/SS7-38
> https://issues.asterisk.org/jira/browse/SS7-39
> https://issues.asterisk.org/jira/browse/SS7-40
> https://issues.asterisk.org/jira/browse/SS7-42
> https://issues.asterisk.org/jira/browse/SS7-43
> https://issues.asterisk.org/jira/browse/SS7-45
> https://issues.asterisk.org/jira/browse/SS7-46
> https://issues.asterisk.org/jira/browse/SS7-47
> https://issues.asterisk.org/jira/browse/SS7-48
> https://issues.asterisk.org/jira/browse/SS7-49
> https://issues.asterisk.org/jira/browse/SS7-51
> https://issues.asterisk.org/jira/browse/SS7-52
> https://issues.asterisk.org/jira/browse/SS7-53
> https://issues.asterisk.org/jira/browse/SS7-54
> https://issues.asterisk.org/jira/browse/SS7-7
>
>
> Diffs
> -----
>
> branches/1.0/ss7_internal.h 300
> branches/1.0/ss7linktest.c 300
> branches/1.0/libss7.h 300
> branches/1.0/mtp2.h 300
> branches/1.0/mtp2.c 300
> branches/1.0/mtp3.h 300
> branches/1.0/mtp3.c 300
> branches/1.0/parser_debug.c 300
> branches/1.0/ss7.c 300
> branches/1.0/isup.c 300
> branches/1.0/isup.h 300
> branches/1.0/ss7test.c 300
>
> Diff: https://reviewboard.asterisk.org/r/1653/diff
>
>
> Testing
> -------
>
> compiles, link setup, cli commands, bassic calls, connected line and redirection
>
>
> Thanks,
>
> KNK
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120719/b9c24f8f/attachment.htm>
More information about the asterisk-dev
mailing list