<p>Patch set 1:<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/12134">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/12134/1//COMMIT_MSG">Commit Message:</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/12134/1//COMMIT_MSG@11">Patch Set #1, Line 11:</a> <code style="font-family:monospace,monospace">command-line counterparts with default value were silently ignored.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Good Catch, it took me a while to understand and see the issue.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12134/1//COMMIT_MSG@13">Patch Set #1, Line 13:</a> <code style="font-family:monospace,monospace">Let's fix this by making config file values to be always preferred over</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't like this fixing approach, it makes no sense. I'd rather drop cmd line args and supporting only cfg file than having cfg file prcede cmdline. That's super non intuitive, goes against usual behavior of apps and the opposite the user expects.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Best fix would be to remove "default" keyword from argparser and apply it manually if the value is None after applying the cfg option from cfg file.</p><p style="white-space: pre-wrap; word-wrap: break-word;">See https://docs.python.org/2/library/argparse.html#default</p><p style="white-space: pre-wrap; word-wrap: break-word;">if default keyword is not provided, then default=None, so that works easily with what we already have and what I propose.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12134">change 12134</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/12134"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: python/osmo-python-tests </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I471b5a6497eadce6456e835233fdaba88a593324 </div>
<div style="display:none"> Gerrit-Change-Number: 12134 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 05 Dec 2018 13:24:45 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>