<p style="white-space: pre-wrap; word-wrap: break-word;">hmm, if stsp also agrees, maybe I should insist on my hunch that a generic T1234 dynamic setting will not make sense anyway. But I don't want to block this more than necessary.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Arguing for my earlier points... just a +1 from me for now because I think they are valid points.</p><p>Patch set 4:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +1</span></p><p><a href="https://gerrit.osmocom.org/11654">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c">File src/osmo-bsc/bsc_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/11654/3/src/osmo-bsc/bsc_vty.c@3916">Patch Set #3, Line 3916:</a> <code style="font-family:monospace,monospace"> "T-number, optionally preceded by 't' or 'T'.")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I didn't completely get what you mean here regarding docs, but anyway that sounds confusing to me wh […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Point 1 is, we often have #defines to re-use the exact same string again, because only one doc string survives in the online docs.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> DEFUN("foo one", "Foo to set one\n" "One arg")<br> DEFUN("foo two", "Foo to set two\n" "Two")</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> > list<br> foo one<br> foo two<br> > foo?<br> Foo to set one<br> > foo ?<br> one One arg<br> two Two</pre><p style="white-space: pre-wrap; word-wrap: break-word;">So in that case, you want a FOO_STR that is always the same.</p><p style="white-space: pre-wrap; word-wrap: break-word;">But when there is a 'no' before that, that's not an issue.<br>This example shows that, and also illustrates the other point:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> #define FOO_STR "Set a foo value\n"<br> DEFUN("foo one", FOO_STR "One arg")<br> DEFUN("no foo", NO_STR FOO_STR)</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> > list<br> foo one<br> no foo<br> > foo?<br> Set a foo value<br> > no?<br> Negate a command or set its defaults<br> > no ?<br> foo Set a foo value<br> > no foo?<br> Set a foo value</pre><p style="white-space: pre-wrap; word-wrap: break-word;">"no foo" now says that it sets a value. For me it makes more sense to describe what "no foo" would actually do:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> DEFUN("no foo", NO_STR "Disable foo")</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> > no?<br> Negate a command or set its defaults<br> > no ?<br> foo Disable foo<br> > no foo?<br> Disable foo</pre><p style="white-space: pre-wrap; word-wrap: break-word;">So</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> > no timer-dynamic?<br> Calculate T3113 dynamically based on channel config and load</pre><p style="white-space: pre-wrap; word-wrap: break-word;">that's "WTF" to me.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11654/3/src/osmo-bsc/bsc_vty.c@3927">Patch Set #3, Line 3927:</a> <code style="font-family:monospace,monospace"> return CMD_WARNING;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Sure, but anyway nowadays doesn't make sense, to let's better tell the user probably he's doing some […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">(still think it makes sense to say "has no dynamic" when the user tries to set a dynamic value; if you really need to say something here, maybe rather "T%d already is non-dynamic"?)</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11654">change 11654</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/11654"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I4fb2969b690151415038631fb6ad059aa6835c7f </div>
<div style="display:none"> Gerrit-Change-Number: 11654 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </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 <ssperling@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 27 Nov 2018 01:33:55 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>