<p>Patch set 11:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/msc_vty.c">File src/libmsc/msc_vty.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/msc_vty.c@566">Patch Set #11, Line 566:</a> <code style="font-family:monospace,monospace">rnc</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In the commit message you state that RNC is not responsible for paging, and it's the task of the MSC itself, so why do we have this prefix here? I find this a bit confusing, because it looks like a parameter that we somehow indicate to the RNC. Let's better use RAN type here as a prefix - 'utran' or '3g'.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/msc_vty.c@575">Patch Set #11, Line 575:</a> <code style="font-family:monospace,monospace"> {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">cosmetic: drop curly braces</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/msc_vty.c@582">Patch Set #11, Line 582:</a> <code style="font-family:monospace,monospace">response-timer</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I believe we should move to generic timer configuration commands, and we already have them in the 'msc' section:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  timer [(vlr|mgw|mncc|sccp|geran|utran|sgs)] [TNNNN] [(<0-2147483647>|default)]</pre><p style="white-space: pre-wrap; word-wrap: break-word;">this one would go to 'utran'. We just need to assign a new TNNNN.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I know that we already have 'paging response-timer' for GERAN as a separate command, but it does not mean that we should populate the timer commands zoo with even more different entries. Ideally we need to deprecate this command and move it to 'geran' (see above). The new TNNNN should be the same for both GERAN and UTRAN.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/paging.c">File src/libmsc/paging.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494/11/src/libmsc/paging.c@63">Patch Set #11, Line 63:</a> <code style="font-family:monospace,monospace">LOGL_ERROR</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is it really an ERROR? Maybe rather NOTICE or even INFO?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-msc/+/15494">change 15494</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-msc/+/15494"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I32c47958939a4a29292832289f9d29905731d7f3 </div>
<div style="display:none"> Gerrit-Change-Number: 15494 </div>
<div style="display:none"> Gerrit-PatchSet: 11 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: efistokl <mykola@kingmuffin.com> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Sat, 12 Sep 2020 08:12:36 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>