Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1449)

Issue 340100043: [l2tdevtools] Add yapf helper #206 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 2 weeks ago by onager
Modified:
3 months ago
Reviewers:
Joachim Metz, jberggren
CC:
kiddi, log2timeline-dev_googlegroups.com
Visibility:
Public.

Description

[l2tdevtools] Add yapf helper #206

Patch Set 1 #

Total comments: 8

Patch Set 2 : Changes after review #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -37 lines) Patch
M .style.yapf View 1 chunk +3 lines, -1 line 0 comments Download
M l2tdevtools/helpers/review.py View 1 9 chunks +54 lines, -9 lines 0 comments Download
A + l2tdevtools/helpers/yapf.py View 1 3 chunks +28 lines, -22 lines 1 comment Download
A + tests/helpers/yapf.py View 1 1 chunk +5 lines, -5 lines 0 comments Download
M tools/review.py View 1 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 10
onager
5 months, 2 weeks ago (2018-01-04 12:42:42 UTC) #1
Joachim Metz
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#newcode12 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#newcode78 l2tdevtools/helpers/yapf.py:78: if there's ...
5 months, 2 weeks ago (2018-01-04 15:50:57 UTC) #2
jberggren
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#newcode88 l2tdevtools/helpers/yapf.py:88: return None On 2018/01/04 15:50:57, Joachim Metz wrote: > ...
3 months, 3 weeks ago (2018-03-01 01:10:40 UTC) #3
Joachim Metz
> Seeing that this function can return other values I prefer to be explicit and ...
3 months, 3 weeks ago (2018-03-01 14:29:46 UTC) #4
onager
Code updated.
3 months, 2 weeks ago (2018-03-09 15:29:25 UTC) #5
onager
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#newcode12 l2tdevtools/helpers/yapf.py:12: class YapfHelper(cli.CLIHelper): On 2018/01/04 15:50:57, Joachim Metz wrote: > ...
3 months, 2 weeks ago (2018-03-09 15:29:57 UTC) #6
Joachim Metz
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#newcode88 l2tdevtools/helpers/yapf.py:88: return None Let's split this into a separate CL ...
3 months, 1 week ago (2018-03-11 21:11:44 UTC) #7
onager
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#newcode88 l2tdevtools/helpers/yapf.py:88: return None On 2018/03/11 21:11:44, Joachim Metz wrote: > ...
3 months, 1 week ago (2018-03-12 18:08:14 UTC) #8
Joachim Metz
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.py#newcode90 l2tdevtools/helpers/yapf.py:90: return None Please create an issue to track ...
3 months, 1 week ago (2018-03-13 04:12:55 UTC) #9
onager
3 months ago (2018-03-18 10:29:44 UTC) #10
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b