[l2tdevtools] Add yapf helper #206

[l2tdevtools] Add yapf helper #206

[Joachim Metz, 2018-01-04 15:50:57]

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:12: class YapfHelper(cli.CLIHelper):
YAPF? (per https://github.com/log2timeline/plaso/wiki/Style-guide#naming)

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:78: if there's no code style defined.
no code style =>
no code style configuration
or 
no code style file

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:88: return None
consistency nit: return None =>  return

(we can also switch to return None everywhere)

[jberggren, 2018-03-01 01:10:40]

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:88: return None
On 2018/01/04 15:50:57, Joachim Metz wrote:
> consistency nit: return None =>  return
> 
> (we can also switch to return None everywhere)

Seeing that this function can return other values I prefer to be explicit and do
"return None". If the functions return value is not meant to be used elsewhere I
prefer to just return.

[Joachim Metz, 2018-03-01 14:29:46]

> Seeing that this function can return other values I prefer to be explicit and
do
> "return None". If the functions return value is not meant to be used elsewhere
I
> prefer to just return.

IMHO then we should make this part of the preferred style and change it
everywhere

My preference for readability would be to us the more explicit form (return
None)
but we need to be consistent as well

[onager, 2018-03-09 15:29:25]

Code updated.

[onager, 2018-03-09 15:29:57]

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:12: class YapfHelper(cli.CLIHelper):
On 2018/01/04 15:50:57, Joachim Metz wrote:
> YAPF? (per https://github.com/log2timeline/plaso/wiki/Style-guide#naming)

Done.

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:88: return None
On 2018/03/01 01:10:40, jberggren wrote:
> On 2018/01/04 15:50:57, Joachim Metz wrote:
> > consistency nit: return None =>  return
> > 
> > (we can also switch to return None everywhere)
> 
> Seeing that this function can return other values I prefer to be explicit and
do
> "return None". If the functions return value is not meant to be used elsewhere
I
> prefer to just return.

+1 to jberggren. I'll add this to style guide. Newer versions of pylint enforce
this as well, so we'll catch those there.

[Joachim Metz, 2018-03-11 21:11:44]

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:88: return None
Let's split this into a separate CL / issue and not mix various changes.

[onager, 2018-03-12 18:08:14]

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/1/l2tdevtools/helpers/yapf.py#n...
l2tdevtools/helpers/yapf.py:88: return None
On 2018/03/11 21:11:44, Joachim Metz wrote:
> Let's split this into a separate CL / issue and not mix various changes.

Done.

[Joachim Metz, 2018-03-13 04:12:55]

LGTM

https://codereview.appspot.com/340100043/diff/20001/l2tdevtools/helpers/yapf.py
File l2tdevtools/helpers/yapf.py (right):

https://codereview.appspot.com/340100043/diff/20001/l2tdevtools/helpers/yapf....
l2tdevtools/helpers/yapf.py:90: return None
Please create an issue to track changing the style to "return None"

[onager, 2018-03-18 10:29:44]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: review.py close yapf_helper