<p>Patch set 3:<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/12243">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12243/3/src/gprs/sgsn_vty.c">File src/gprs/sgsn_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/12243/3/src/gprs/sgsn_vty.c@1195">Patch Set #3, Line 1195:</a> <code style="font-family:monospace,monospace">      if (!parsing_config_file) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">instead you can do</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  if (vty->type != VTY_FILE)</pre><p style="white-space: pre-wrap; word-wrap: break-word;">A comment should explain why this is necessary. We have other commands that allow changing the value and write back the config, even though they have no immediate effect on the running program. I think it's good to print a warning, but do you really need to forbid tweaking the config?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12243">change 12243</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/12243"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ib2f65fed9f56b9718e8a9647e3f01dce69870c1f </div>
<div style="display:none"> Gerrit-Change-Number: 12243 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <stsp@stsp.name> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Stefan Sperling <stsp@stsp.name> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 17 Dec 2018 15:09:42 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>