Re: Review Request 58065: Added initial code for the python mesoshttp package.

[Kevin Klues <kl...@gmail.com>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58065/#review171181
-----------------------------------------------------------



One quick comment from glancing through the patch. Why not add the package as mesos/http instead of mesoshttp?
I think we should rename lib/mesos in cli_new to lib/cli. That should be one nice isolated patch.

Also small things like adding packages to the virtualenv and the gitignore file should be separate patches.
The rule of thumb is to group things together that function together, so you can write clear concise commit messages about the change. Honestly, I'd even split the virtualenv changes into two patches. One for the pytest additions explaining how they will be used in a future patch to run unit tests for the cli, and one for the mock explaining its intended usage. It may seem like a lot of "unnecessary" work up front, but it makes it much easier as a reviewer to come in and make a decision right away as to whether a patch is ready to be committed or not.
  
It also makes it easier to track changes and revert them in small isolated chunks if necessary.

- Kevin Klues


On April 6, 2017, 12:13 a.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58065/
> -----------------------------------------------------------
> 
> (Updated April 6, 2017, 12:13 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added initial code for the python mesoshttp package.
> 
> 
> Diffs
> -----
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d40000318f2d0e439a6edf 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesoshttp/__init__.py PRE-CREATION 
>   src/python/lib/mesoshttp/exceptions.py PRE-CREATION 
>   src/python/lib/mesoshttp/http.py PRE-CREATION 
>   src/python/lib/tests/mesoshttp/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/mesoshttp/test_http.py PRE-CREATION 
>   support/mesos-style.py 6489c2de8cc8e14f28ef89c7604c94d3eaaaadff 
> 
> 
> Diff: https://reviews.apache.org/r/58065/diff/1/
> 
> 
> Testing
> -------
> 
> unit tests:
> $ source src/cli_new/activate
> $ pytest --cov src/python/lib/mesoshttp --cov-report term-missing src/python/lib
> 
> 
> Thanks,
> 
> Eric Chung
> 
>