You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2018/07/10 09:51:09 UTC
Re: Review Request 67502: Refactored API functionality into separate
module
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67502/#review205901
-----------------------------------------------------------
Fix it, then Ship it!
Only typos to fix, the commit message should be edited to finish with a period and the `Testing Done` part should be filled even if it's just to say that tests have been done later in the chain.
support/python3/common.py
Lines 37 (patched)
<https://reviews.apache.org/r/67502/#comment288786>
Missing `.`.
support/python3/common.py
Lines 45 (patched)
<https://reviews.apache.org/r/67502/#comment288787>
Missing `.`.
support/python3/common.py
Lines 55 (patched)
<https://reviews.apache.org/r/67502/#comment288791>
Is the space after `%s` necessary?
support/python3/common.py
Lines 100 (patched)
<https://reviews.apache.org/r/67502/#comment288792>
In our Python codebase, we prefer to have the space at the beginning of the next string rather than at the end of the string. Please change the lines 99/100/101 accordingly.
support/python3/common.py
Lines 113 (patched)
<https://reviews.apache.org/r/67502/#comment288793>
Here the comment uses the second person where elsewhere it uses the third person. Can you improve the consistency of the comments by always using the third person?
support/python3/common.py
Lines 139 (patched)
<https://reviews.apache.org/r/67502/#comment288790>
Missing `.`.
support/python3/common.py
Lines 143 (patched)
<https://reviews.apache.org/r/67502/#comment288789>
Missing `.`.
support/python3/common.py
Lines 152 (patched)
<https://reviews.apache.org/r/67502/#comment288794>
We generally set which contributor should take care of the todo, here it should thus be `TODO(dragoshsch)`. Also, s/`if`/`If`.
support/python3/common.py
Lines 155 (patched)
<https://reviews.apache.org/r/67502/#comment288788>
Missing `.`.
support/python3/common.py
Lines 156 (patched)
<https://reviews.apache.org/r/67502/#comment288797>
Space issue mentionned before.
support/python3/common.py
Lines 160 (patched)
<https://reviews.apache.org/r/67502/#comment288795>
We generally set which contributor should take care of the todo, here it should thus be `TODO(dragoshsch)`.
support/python3/common.py
Lines 160 (patched)
<https://reviews.apache.org/r/67502/#comment288796>
We generally set which contributor should take care of the todo, here it should thus be `TODO(dragoshsch)`.
- Armand Grillet
On June 14, 2018, 11:13 p.m., Dragos Schebesch wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67502/
> -----------------------------------------------------------
>
> (Updated June 14, 2018, 11:13 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Refactored API functionality into separate module
>
>
> Diffs
> -----
>
> support/python3/common.py PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67502/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dragos Schebesch
>
>