Presubmit tests: allow 100columns limit for .java files.

Presubmit tests: allow 100columns limit for .java files.

Chrome for Android introduced the requirement for java files.
Android's style guide:
http://source.android.com/source/code-style.html#limit-line-length
defines 100 columns for .java files.
This patch changes the presubmit tests which are also used by chromium's CQ.

TEST=testCannedCheckJavaLongLines

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=122938

[bulach, 2012-02-16 22:00:13]

hi maruel,

thanks for the directions!

this patch passes the tests locally, but I have two issues with the presubmit
itself when trying to upload:
- python2.5 (I have 2.6, should it be ok?)

two tests are failing, but they seem unrelated:
- FAIL: testUpdateCheckout (__main__.ManagedGitWrapperTestCase)

FAIL: testUpdateCheckout (__main__.UnmanagedGitWrapperTestCase)


let me know if there's anything I need to setup..

thanks!

[M-A Ruel, 2012-02-16 22:35:43]

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_canned_che...
File presubmit_canned_checks.py (right):

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_canned_che...
presubmit_canned_checks.py:319: # Line length for java.
Another way:
maxlens = { '.java': 100, '': maxlen }

then inside no_long_lines,
file_maxlen = maxlens.get(file_extension, maxlens[''])
and continue with the local file_maxlen

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_support.py
File presubmit_support.py (right):

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_support.py...
presubmit_support.py:457: def GetFileExt(self):
I'd rather not add _yet another member_. Since it's used only at one place, just
to a endswith() in the canned checks or rsplit('.', 1)[-1], it's fine enough.

[bulach, 2012-02-17 01:25:14]

thanks maruel! another look please?

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_canned_che...
File presubmit_canned_checks.py (right):

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_canned_che...
presubmit_canned_checks.py:319: # Line length for java.
On 2012/02/16 22:35:43, Marc-Antoine Ruel wrote:
> Another way:
> maxlens = { '.java': 100, '': maxlen }
> 
> then inside no_long_lines,
> file_maxlen = maxlens.get(file_extension, maxlens[''])
> and continue with the local file_maxlen

smart, thanks!
done.

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_support.py
File presubmit_support.py (right):

https://chromiumcodereview.appspot.com/9417023/diff/2001/presubmit_support.py...
presubmit_support.py:457: def GetFileExt(self):
On 2012/02/16 22:35:43, Marc-Antoine Ruel wrote:
> I'd rather not add _yet another member_. Since it's used only at one place,
just
> to a endswith() in the canned checks or rsplit('.', 1)[-1], it's fine enough.

sounds reasonable. removed..

[bulach, 2012-02-21 23:27:34]

ping? :)

[M-A Ruel, 2012-02-22 00:34:39]

lgtm

http://codereview.chromium.org/9417023/diff/3005/presubmit_canned_checks.py
File presubmit_canned_checks.py (right):

http://codereview.chromium.org/9417023/diff/3005/presubmit_canned_checks.py#n...
presubmit_canned_checks.py:316: long_symbol = maxlen * 2 / 3
You probably want to define these locally to the function. Or you may not care
about long symbols or urls in java. I don't mind much.

[bulach, 2012-02-22 00:55:37]

thanks maruel! addressed the last comment, another look please?

a few questions:
1. does the CQ work for this patch?
2. once it lands, does the chromium CQ automatically sync it?

http://codereview.chromium.org/9417023/diff/3005/presubmit_canned_checks.py
File presubmit_canned_checks.py (right):

http://codereview.chromium.org/9417023/diff/3005/presubmit_canned_checks.py#n...
presubmit_canned_checks.py:316: long_symbol = maxlen * 2 / 3
On 2012/02/22 00:34:39, Marc-Antoine Ruel wrote:
> You probably want to define these locally to the function. Or you may not care
> about long symbols or urls in java. I don't mind much.

good point, done.

[M-A Ruel, 2012-02-22 01:02:18]

On 2012/02/22 00:55:37, bulach wrote:
> thanks maruel! addressed the last comment, another look please?
> 
> a few questions:
> 1. does the CQ work for this patch?

YES.

> 2. once it lands, does the chromium CQ automatically sync it?

i DON'T UNDERSTAND THE QUESTION? DEPOT_TOOLS AUTOMATICALLY SYNCS ITSELF AT EACH
GCLIENT INVOCATION.

M-A

(MY CHROMEBOOK IS STUCK ON ALLCAPS)

[bulach, 2012-02-22 01:05:41]

On 2012/02/22 01:02:18, Marc-Antoine Ruel wrote:
> On 2012/02/22 00:55:37, bulach wrote:
> > thanks maruel! addressed the last comment, another look please?
> > 
> > a few questions:
> > 1. does the CQ work for this patch?
> 
> YES.
> 
> > 2. once it lands, does the chromium CQ automatically sync it?
> 
> i DON'T UNDERSTAND THE QUESTION? DEPOT_TOOLS AUTOMATICALLY SYNCS ITSELF AT
EACH
> GCLIENT INVOCATION.

hahaha!! cool, THANKS! ;)
(last week you mentioned you had to restart the CQ for that svn properties,
looks like there's no need for this one...).

I'm adding this to the CQ now..

> 
> M-A
> 
> (MY CHROMEBOOK IS STUCK ON ALLCAPS)

[commit-bot: I haz the power, 2012-02-22 01:06:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bulach@chromium.org/9417023/10001

[commit-bot: I haz the power, 2012-02-22 01:13:34]

Change committed as 122938