Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12134 )
Change subject: ctrl2cgi: fix broken config override ...................................................................... Patch Set 1: Code-Review-1 (2 comments) https://gerrit.osmocom.org/#/c/12134/1//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/12134/1//COMMIT_MSG@11 PS1, Line 11: command-line counterparts with default value were silently ignored. Good Catch, it took me a while to understand and see the issue. https://gerrit.osmocom.org/#/c/12134/1//COMMIT_MSG@13 PS1, Line 13: Let's fix this by making config file values to be always preferred over 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. 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. See https://docs.python.org/2/library/argparse.html#default if default keyword is not provided, then default=None, so that works easily with what we already have and what I propose. -- To view, visit https://gerrit.osmocom.org/12134 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: python/osmo-python-tests Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I471b5a6497eadce6456e835233fdaba88a593324 Gerrit-Change-Number: 12134 Gerrit-PatchSet: 1 Gerrit-Owner: Max <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Wed, 05 Dec 2018 13:24:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
