----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57896/#review169969 -----------------------------------------------------------
Thanks for the review! I think this review should actualy be broken up into three different reviews though. We typically like to keep each patch doing exactly one thing and I see three logical things going on in this patch. 1) Rename config.py to settings.py in the new CLI. 2) Move new CLI settings out of settings.py into a user-defined TOML file. 3) Introduce a 'config' plugin to the new CLI. This way you can add some descriptions about each of these logical changes in the descriptions of the commit messages. In the meantime, I've left some other high level comments on the current review. I'll dig into the rest of it once it is broken up properly. src/cli_new/README.md Lines 46-67 (patched) <https://reviews.apache.org/r/57896/#comment242690> I don't think we should introduce any of that in this commit. These should only be added once as part of commits that actually use this functionality. In master right now, these configs are currently meaningless and shouldn't appear here yet. The only thing that should appear here currently is `plugins`. src/cli_new/README.md Lines 87 (patched) <https://reviews.apache.org/r/57896/#comment242691> Why `MESOS_CLI_CONFIG` instead of `MESOS_CLI_CONFIG_DIR`? Is the idea that we could keep multiple configs in this directory and swap them out based on the environment variable? - Kevin Klues On March 23, 2017, 11:05 p.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57896/ > ----------------------------------------------------------- > > (Updated March 23, 2017, 11:05 p.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-7269 > https://issues.apache.org/jira/browse/MESOS-7269 > > > Repository: mesos > > > Description > ------- > > Documentation added to create a correct configuration file. > Plugin to handle this configuration file added. > config.py moved to settings.py to differentiate it from the plugin. > > > Diffs > ----- > > src/cli_new/README.md 0e60515b71192ce1a544711948a5c17a6f9002af > src/cli_new/bin/config.py 274f8c63b0c642637f17aa2e3c8c4a8a5a059e37 > src/cli_new/bin/main.py bbfb52c894540158c70e0f50ebb8a277b692d54d > src/cli_new/bin/settings.py PRE-CREATION > src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION > src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION > src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d40000318f2d0e439a6edf > > > Diff: https://reviews.apache.org/r/57896/diff/1/ > > > Testing > ------- > > Tested manually, PEP8 and Pylint used to make sure that the code style is > correct. > > > Thanks, > > Armand Grillet > >
