[Asterisk-code-review] res pjsip: Re-use IP version of signaling (SIP) for media (R... (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: res_pjsip: Re-use IP version of signaling (SIP) for media (RTP).
......................................................................


Patch Set 2: Code-Review+2

> > might want to
 > 
 > No intention from my site. My resources are limited.
 > 
 > > bunch of places
 > 
 > Please, be as concrete as possible. This eases understanding for
 > others and helps to avoid overlooks. The code appears 3+2 times.
 > 
 > > make it an inline in res_pjsip.h
 > 
 > Being an external contributor, please, be as concrete as possible.
 > For example, where to place that static inline function (you did
 > that), how to name the function, and which parameters to use. Even
 > a first shot, a gut feeling helps because it is not only a starting
 > point but can avoid review ping-pong as well. If feasible, explain
 > why you have chosen something. For example, here in this case, I
 > guess you want to place this inline function in the
 > lowest-common-denominator header, the most specific one. That way I
 > understand your advice as example but do not follow blindly because
 > I double-check for myself (which adds a good, no-overhead,
 > invisible review circle). In this case, I would have gone for
 > "asterisk/res_pjsip_session.h" instead.

Ok, I get your point. :)

-- 
To view, visit https://gerrit.asterisk.org/3663
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I01a85a8c6723fcc12e86139f80e090e2078d04bb
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: No