Add Pylint for docs server.

Add Pylint for docs server.

BUG=434363
TEST=git cl presubmit -uv
R=kalman@chromium.org

Committed: https://crrev.com/202524aacb06c948a3f4a34f4418f32b550c79b5
Cr-Commit-Position: refs/heads/master@{#317721}

[tfarina, 2015-02-21 17:14:59]

PTAL

[not at google - send to devlin, 2015-02-23 18:43:30]

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/PRESUBMIT.py:97: return results
Could you leave the formatting as before?

[tfarina, 2015-02-23 20:17:55]

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/PRESUBMIT.py:97: return results
On 2015/02/23 18:43:30, kalman wrote:
> Could you leave the formatting as before?

Any reason why? I assume you are talking about:

return ( ... + ... ) 

vs

results += 

?

[not at google - send to devlin, 2015-02-23 20:34:19]

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/PRESUBMIT.py:97: return results
On 2015/02/23 20:17:55, tfarina wrote:
> On 2015/02/23 18:43:30, kalman wrote:
> > Could you leave the formatting as before?
> 
> Any reason why? I assume you are talking about:
> 
> return ( ... + ... ) 
> 
> vs
> 
> results += 
> 
> ?

Yes. The reason is because it's an unnecessary cosmetic change.

[tfarina, 2015-02-23 20:39:21]

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/PRESUBMIT.py:97: return results
On 2015/02/23 20:34:19, kalman wrote:
> On 2015/02/23 20:17:55, tfarina wrote:
> > On 2015/02/23 18:43:30, kalman wrote:
> > > Could you leave the formatting as before?
> > 
> > Any reason why? I assume you are talking about:
> > 
> > return ( ... + ... ) 
> > 
> > vs
> > 
> > results += 
> > 
> > ?
> 
> Yes. The reason is because it's an unnecessary cosmetic change.

Indeed! Done.

[not at google - send to devlin, 2015-02-23 20:41:57]

Thanks! LGTM, though, how many warnings does this introduce?

[tfarina, 2015-02-23 20:50:01]

It generated this: http://pastebin.com/VVrjRYbH

[not at google - send to devlin, 2015-02-23 20:52:35]

On 2015/02/23 20:50:01, tfarina wrote:
> It generated this: http://pastebin.com/VVrjRYbH

Oh... that's a lot. Ideally we'd fix those all, it's not great having an epic
list of warnings block upload now.

[not at google - send to devlin, 2015-02-23 20:52:44]

On 2015/02/23 20:52:35, kalman wrote:
> On 2015/02/23 20:50:01, tfarina wrote:
> > It generated this: http://pastebin.com/VVrjRYbH
> 
> Oh... that's a lot. Ideally we'd fix those all, it's not great having an epic
> list of warnings block upload now.

Or make it just informational until we do fix them.

[tfarina, 2015-02-23 21:05:22]

Do you want me to create a disabled_warnings list and/or a black_list?

[not at google - send to devlin, 2015-02-23 21:09:05]

On 2015/02/23 21:05:22, tfarina wrote:
> Do you want me to create a disabled_warnings list and/or a black_list?

Sounds like too much effort... I thought there was just a way of adding
presubmit messages (which don't prevent upload), vs warnings (which do)?

[tfarina, 2015-02-23 21:13:19]

On 2015/02/23 21:09:05, kalman wrote:
> On 2015/02/23 21:05:22, tfarina wrote:
> > Do you want me to create a disabled_warnings list and/or a black_list?
> 
> Sounds like too much effort... I thought there was just a way of adding
> presubmit messages (which don't prevent upload), vs warnings (which do)?

That is what will happen. On upload, the dev will get only a warning. Only on
committing it will be an error.

https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/presubm...

[not at google - send to devlin, 2015-02-23 21:16:45]

On 2015/02/23 21:13:19, tfarina wrote:
> On 2015/02/23 21:09:05, kalman wrote:
> > On 2015/02/23 21:05:22, tfarina wrote:
> > > Do you want me to create a disabled_warnings list and/or a black_list?
> > 
> > Sounds like too much effort... I thought there was just a way of adding
> > presubmit messages (which don't prevent upload), vs warnings (which do)?
> 
> That is what will happen. On upload, the dev will get only a warning. Only on
> committing it will be an error.
> 
>
https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/presubm...

Yes, preventing commit sounds annoying, especially since it'll prevent bots from
committing.

[tfarina, 2015-02-23 21:23:56]

On Mon, Feb 23, 2015 at 6:16 PM, <kalman@chromium.org> wrote:

> On 2015/02/23 21:13:19, tfarina wrote:
>
>> On 2015/02/23 21:09:05, kalman wrote:
>> > On 2015/02/23 21:05:22, tfarina wrote:
>> > > Do you want me to create a disabled_warnings list and/or a black_list?
>> >
>> > Sounds like too much effort... I thought there was just a way of adding
>> > presubmit messages (which don't prevent upload), vs warnings (which do)?
>>
>
>  That is what will happen. On upload, the dev will get only a warning.
>> Only on
>> committing it will be an error.
>>
>
>
> https://chromium.googlesource.com/chromium/tools/depot_
> tools/+/master/presubmit_canned_checks.py#724
>
> Yes, preventing commit sounds annoying, especially since it'll prevent
> bots from
> committing.
>
I'm not sure that is what is going to happen.

If I'm not mistaken it would prevent a dev from landing manually, but not
the CQ.

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[not at google - send to devlin, 2015-02-23 21:31:06]

On 2015/02/23 21:23:56, tfarina wrote:
> On Mon, Feb 23, 2015 at 6:16 PM, <mailto:kalman@chromium.org> wrote:
> 
> > On 2015/02/23 21:13:19, tfarina wrote:
> >
> >> On 2015/02/23 21:09:05, kalman wrote:
> >> > On 2015/02/23 21:05:22, tfarina wrote:
> >> > > Do you want me to create a disabled_warnings list and/or a black_list?
> >> >
> >> > Sounds like too much effort... I thought there was just a way of adding
> >> > presubmit messages (which don't prevent upload), vs warnings (which do)?
> >>
> >
> >  That is what will happen. On upload, the dev will get only a warning.
> >> Only on
> >> committing it will be an error.
> >>
> >
> >
> > https://chromium.googlesource.com/chromium/tools/depot_
> > tools/+/master/presubmit_canned_checks.py#724
> >
> > Yes, preventing commit sounds annoying, especially since it'll prevent
> > bots from
> > committing.
> >
> I'm not sure that is what is going to happen.
> 
> If I'm not mistaken it would prevent a dev from landing manually, but not
> the CQ.
> 
> -- 
> Thiago Farina
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

How about you just print() these for now, and then we can work to fix them, then
change the print() into a warning? A hacky solution but it gets the ball
rolling.

[tfarina, 2015-02-23 21:33:04]

On 2015/02/23 21:31:06, kalman wrote:
> How about you just print() these for now, and then we can work to fix them,
then
> change the print() into a warning? A hacky solution but it gets the ball
> rolling.
How you want me to do that? Do I keep running RunPylint? Sorry, I'm not super
familiar with Python.

[not at google - send to devlin, 2015-02-23 21:54:10]

On 2015/02/23 21:33:04, tfarina wrote:
> On 2015/02/23 21:31:06, kalman wrote:
> > How about you just print() these for now, and then we can work to fix them,
> then
> > change the print() into a warning? A hacky solution but it gets the ball
> > rolling.
> How you want me to do that? Do I keep running RunPylint? Sorry, I'm not super
> familiar with Python.

In _RunPresubmit(), write:

_BuildServer(input_api)
# For now, print any lint errors. When these have been eliminated,
# move these into the warning list below.
# See crbug.com/...
lint_errors =  input_api.canned_checks.RunPylint(input_api, output_api)
if lint_errors:
  print(lint_errors.join('\n'))
return ...

[tfarina, 2015-02-24 00:47:51]

Done. PTAL

[not at google - send to devlin, 2015-02-24 01:01:25]

lgtm

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/server2/PRESUBMIT.py:93: print(lint_errors)
if you don't do the ''.join thing then it'll format it as a list rather than on
every line, which will look silly.

[tfarina, 2015-02-24 01:05:35]

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/server2/PRESUBMIT.py:93: print(lint_errors)
On 2015/02/24 01:01:25, kalman wrote:
> if you don't do the ''.join thing then it'll format it as a list rather than
on
> every line, which will look silly.
it was not possible to do a join in the list:
AttributeError: 'list' object has no attribute 'join'

The current output is: http://pastebin.com/Cz9xeidV

[tfarina, 2015-02-24 01:05:45]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2015-02-24 01:06:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/942263002/60001

[not at google - send to devlin, 2015-02-24 01:07:30]

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
chrome/common/extensions/docs/server2/PRESUBMIT.py:93: print(lint_errors)
On 2015/02/24 01:05:35, tfarina wrote:
> On 2015/02/24 01:01:25, kalman wrote:
> > if you don't do the ''.join thing then it'll format it as a list rather than
> on
> > every line, which will look silly.
> it was not possible to do a join in the list:
> AttributeError: 'list' object has no attribute 'join'
> 
> The current output is: http://pastebin.com/Cz9xeidV

If I said lint_errors.join('') before, I meant ''.join(lint_errors).

[not at google - send to devlin, 2015-02-24 01:07:47]

not lgtm please fix before submitting

[tfarina, 2015-02-24 01:13:12]

On 2015/02/24 01:07:30, kalman wrote:
>
https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
> File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):
> 
>
https://codereview.chromium.org/942263002/diff/60001/chrome/common/extensions...
> chrome/common/extensions/docs/server2/PRESUBMIT.py:93: print(lint_errors)
> On 2015/02/24 01:05:35, tfarina wrote:
> > On 2015/02/24 01:01:25, kalman wrote:
> > > if you don't do the ''.join thing then it'll format it as a list rather
than
> > on
> > > every line, which will look silly.
> > it was not possible to do a join in the list:
> > AttributeError: 'list' object has no attribute 'join'
> > 
> > The current output is: http://pastebin.com/Cz9xeidV
> 
> If I said lint_errors.join('') before, I meant ''.join(lint_errors).

TypeError: sequence item 0: expected string, _PresubmitPromptWarning found

Looks like mixing String and Lists in Python do not work very well.

[commit-bot: I haz the power, 2015-02-24 01:27:40]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-24 01:28:21]

Patchset 4 (id:??) landed as
https://crrev.com/202524aacb06c948a3f4a34f4418f32b550c79b5
Cr-Commit-Position: refs/heads/master@{#317721}