Prase IDL files in common/extensions/api PRESUBMIT check.

Prase IDL files in common/extensions/api PRESUBMIT check.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224709

[Haojian Wu, 2013-09-09 11:09:37]

yoz@, kalman@ for correctness.
maruel@ for pythonness.

[M-A Ruel, 2013-09-09 14:00:11]

On 2013/09/09 11:09:37, Haojian Wu wrote:
> yoz@, kalman@ for correctness.
> maruel@ for pythonness.

python lgtm

[not at google - send to devlin, 2013-09-09 16:14:07]

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:58: filename =
affected_file.AbsoluteLocalPath()
too much indent

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:62: 'Features file %s could not be
parsed: %s' %
s/Features/IDL/

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:63: (affected_file.LocalPath(),
parse_error)))
You could factor this more concisely:

actions = {
  '.json': _GetJSONParseError,
  '.idl':  _GetIDLParseError
}
def get_action(filename):
  return actions.get(os.path.splitext(filename)[1])

for affected_file in input_api.AffectedFiles(file_filter=get_action):
  filename = affected_file.AbsoluteLocalPath()
  parse_error = get_action(affected_file)(input_api,
                                          filename,
                                          input_api.ReadFile(filename))
  if parse_error:
    results.append(output_api.PresubmitError('%s could not be parsed: %s' %
        (affected_file.LocalPath(), parse_error))

then factor _GetJSONParseError and _GetIDLParseError to match. It looks like
they share a lot of structure too, so you could do some cleanup there.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_1.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_1.idl:8: /* Unexpected
"{" after keyword "dictionary". */
for all of these test file, use // not /* */.

however for all of these, I'd change

// Tests an invalid IDL file.

namespace test {
  /* some_condition */
  ...
}

to

// Tests some_condition.
namespace test {
}

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl:8: /* Unexpected
symbol DOMString afte symbol a. */
s/afte/after/

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl:14: 
no \n

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_7.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_7.idl:14: 
no blank lines

[Yoyo Zhou, 2013-09-09 17:19:27]

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/PRESUBMIT_test.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT_test.py:84: filename =
'test_presubmit/invalid_idl_' + str(i+1) + '.idl'
Why not simply
for filename in glob.glob(...)

[Haojian Wu, 2013-09-10 02:51:04]

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:58: filename =
affected_file.AbsoluteLocalPath()
On 2013/09/09 16:14:07, kalman wrote:
> too much indent

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:62: 'Features file %s could not be
parsed: %s' %
On 2013/09/09 16:14:07, kalman wrote:
> s/Features/IDL/

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT.py:63: (affected_file.LocalPath(),
parse_error)))
On 2013/09/09 16:14:07, kalman wrote:
> You could factor this more concisely:
> 
> actions = {
>   '.json': _GetJSONParseError,
>   '.idl':  _GetIDLParseError
> }
> def get_action(filename):
>   return actions.get(os.path.splitext(filename)[1])
> 
> for affected_file in input_api.AffectedFiles(file_filter=get_action):
>   filename = affected_file.AbsoluteLocalPath()
>   parse_error = get_action(affected_file)(input_api,
>                                           filename,
>                                           input_api.ReadFile(filename))
>   if parse_error:
>     results.append(output_api.PresubmitError('%s could not be parsed: %s' %
>         (affected_file.LocalPath(), parse_error))
> 
> then factor _GetJSONParseError and _GetIDLParseError to match. It looks like
> they share a lot of structure too, so you could do some cleanup there.

Done. Thanks for the detailed instructions!

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/PRESUBMIT_test.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/PRESUBMIT_test.py:84: filename =
'test_presubmit/invalid_idl_' + str(i+1) + '.idl'
On 2013/09/09 17:19:27, Yoyo Zhou wrote:
> Why not simply
> for filename in glob.glob(...)

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_1.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_1.idl:8: /* Unexpected
"{" after keyword "dictionary". */
On 2013/09/09 16:14:07, kalman wrote:
> for all of these test file, use // not /* */.
> 
> however for all of these, I'd change
> 
> // Tests an invalid IDL file.
> 
> namespace test {
>   /* some_condition */
>   ...
> }
> 
> to
> 
> // Tests some_condition.
> namespace test {
> }

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl:8: /* Unexpected
symbol DOMString afte symbol a. */
On 2013/09/09 16:14:07, kalman wrote:
> s/afte/after/

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_2.idl:14: 
On 2013/09/09 16:14:07, kalman wrote:
> no \n

Done.

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/test_presubmit/invalid_idl_7.idl (right):

https://chromiumcodereview.appspot.com/23512007/diff/1/chrome/common/extensio...
chrome/common/extensions/api/test_presubmit/invalid_idl_7.idl:14: 
On 2013/09/09 16:14:07, kalman wrote:
> no blank lines

Done.

[not at google - send to devlin, 2013-09-10 22:51:20]

lgtm with those changes.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:35: return None
you could just have

return error or None

here.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:52: import os
just import os at the top with the other imports

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT_test.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:29: return s
just return f.read()?

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:47: for (offset, filename) in
enumerate(
you don't need the parens around this. however, a more concise way to write
this:

actual_errors = [
    PRESUBMIT._GetJSONParseError(input_api, filename)
    for filename in sorted(glob('test_presubmit/invalid_*.json'))
]
self.assertEqual(expected_errors, actual_errors)

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:77:
self.assertTrue(expected_errors[offset] in error)
ditto

[Haojian Wu, 2013-09-11 01:10:22]

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:35: return None
On 2013/09/10 22:51:21, kalman wrote:
> you could just have
> 
> return error or None
> 
> here.

Done.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:52: import os
On 2013/09/10 22:51:21, kalman wrote:
> just import os at the top with the other imports

Done.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT_test.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:29: return s
On 2013/09/10 22:51:21, kalman wrote:
> just return f.read()?

Done.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:47: for (offset, filename) in
enumerate(
On 2013/09/10 22:51:21, kalman wrote:
> you don't need the parens around this. however, a more concise way to write
> this:
> 
> actual_errors = [
>     PRESUBMIT._GetJSONParseError(input_api, filename)
>     for filename in sorted(glob('test_presubmit/invalid_*.json'))
> ]
> self.assertEqual(expected_errors, actual_errors)

Done.

https://chromiumcodereview.appspot.com/23512007/diff/12002/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT_test.py:77:
self.assertTrue(expected_errors[offset] in error)
On 2013/09/10 22:51:21, kalman wrote:
> ditto

Done.

[M-A Ruel, 2013-09-11 13:37:58]

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:5: import os
Not necessary

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:53: filename =
affected_file.AbsoluteLocalPath()
FYI, (optional)
filename = affected_file.LocalPath()
would be fine.

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:54: return
actions.get(os.path.splitext(filename)[1])
input_api.os_path.splitext()

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:63: (affected_file.LocalPath(),
parse_error)))
weird alignment

[Haojian Wu, 2013-09-11 14:48:57]

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:5: import os
On 2013/09/11 13:37:59, M-A Ruel wrote:
> Not necessary

Done.

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:53: filename =
affected_file.AbsoluteLocalPath()
On 2013/09/11 13:37:59, M-A Ruel wrote:
> FYI, (optional)
> filename = affected_file.LocalPath()
> would be fine.

Done.

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:54: return
actions.get(os.path.splitext(filename)[1])
On 2013/09/11 13:37:59, M-A Ruel wrote:
> input_api.os_path.splitext()

Done.

https://chromiumcodereview.appspot.com/23512007/diff/25001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:63: (affected_file.LocalPath(),
parse_error)))
On 2013/09/11 13:37:59, M-A Ruel wrote:
> weird alignment

Done.

[M-A Ruel, 2013-09-11 14:54:51]

still lgtm

https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:46: '.idl': _GetIDLParseError
add a coma, and sort the keys.

[Haojian Wu, 2013-09-11 15:20:05]

https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:46: '.idl': _GetIDLParseError
On 2013/09/11 14:54:51, M-A Ruel wrote:
> add a coma, and sort the keys.

Done.

[Haojian Wu, 2013-09-12 00:06:03]

On 2013/09/11 15:20:05, Haojian Wu wrote:
>
https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
> File chrome/common/extensions/api/PRESUBMIT.py (right):
> 
>
https://chromiumcodereview.appspot.com/23512007/diff/30001/chrome/common/exte...
> chrome/common/extensions/api/PRESUBMIT.py:46: '.idl': _GetIDLParseError
> On 2013/09/11 14:54:51, M-A Ruel wrote:
> > add a coma, and sort the keys.
> 
> Done.

ping yoz@.

[Yoyo Zhou, 2013-09-12 18:57:57]

LGTM

[commit-bot: I haz the power, 2013-09-13 00:18:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/31018

[commit-bot: I haz the power, 2013-09-13 00:34:17]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-09-13 10:06:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/49001

[commit-bot: I haz the power, 2013-09-13 10:18:37]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Haojian Wu, 2013-09-13 10:28:06]

On 2013/09/13 10:18:37, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

kalman@, this patch break the presubmit.
Refer to buildbot stdio log, we add checking valid IDL/JSON files, but test
files of this patch are all invalid IDL/JSON files which makes the presubmit
failed.

Is there any solution to fix this?

[not at google - send to devlin, 2013-09-13 16:16:13]

Move the test files into a test/ directory and exclude that from the presubmit.

[Yoyo Zhou, 2013-09-13 16:45:37]

On 2013/09/13 16:16:13, kalman wrote:
> Move the test files into a test/ directory and exclude that from the
presubmit.

Change file_filter to exclude the test_presubmit directory.

[Haojian Wu, 2013-09-14 08:13:10]

On 2013/09/13 16:45:37, Yoyo Zhou wrote:
> On 2013/09/13 16:16:13, kalman wrote:
> > Move the test files into a test/ directory and exclude that from the
> presubmit.
> 
> Change file_filter to exclude the test_presubmit directory.

Done. Thanks.

[Haojian Wu, 2013-09-14 08:13:24]

https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:53: for affected_file in filter(lambda
f: "test_presubmit" not in f.LocalPath(),
Here, please review it again.

[Haojian Wu, 2013-09-17 00:38:29]

On 2013/09/14 08:13:24, Haojian Wu wrote:
>
https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
> File chrome/common/extensions/api/PRESUBMIT.py (right):
> 
>
https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
> chrome/common/extensions/api/PRESUBMIT.py:53: for affected_file in
filter(lambda
> f: "test_presubmit" not in f.LocalPath(),
> Here, please review it again.

kalman@, yoz@, any comments about it?

[Yoyo Zhou, 2013-09-17 19:50:20]

https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/23512007/diff/58001/chrome/common/exte...
chrome/common/extensions/api/PRESUBMIT.py:53: for affected_file in filter(lambda
f: "test_presubmit" not in f.LocalPath(),
On 2013/09/14 08:13:25, Haojian Wu wrote:
> Here, please review it again.

I'd prefer to not have an additional filter here, say
file_filter=lambda f: "test_presubmit" not in f.LocalPath() and get_action(f)

[Haojian Wu, 2013-09-18 00:37:34]

https://codereview.chromium.org/23512007/diff/58001/chrome/common/extensions/...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://codereview.chromium.org/23512007/diff/58001/chrome/common/extensions/...
chrome/common/extensions/api/PRESUBMIT.py:53: for affected_file in filter(lambda
f: "test_presubmit" not in f.LocalPath(),
On 2013/09/17 19:50:20, Yoyo Zhou wrote:
> On 2013/09/14 08:13:25, Haojian Wu wrote:
> > Here, please review it again.
> 
> I'd prefer to not have an additional filter here, say
> file_filter=lambda f: "test_presubmit" not in f.LocalPath() and get_action(f)

Done.

[Yoyo Zhou, 2013-09-18 01:04:30]

LGTM

[commit-bot: I haz the power, 2013-09-18 01:19:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/65001

[commit-bot: I haz the power, 2013-09-18 01:32:49]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Haojian Wu, 2013-09-18 02:00:37]

https://codereview.chromium.org/23512007/diff/76001/chrome/common/extensions/...
File chrome/common/extensions/api/PRESUBMIT_test.py (right):

https://codereview.chromium.org/23512007/diff/76001/chrome/common/extensions/...
chrome/common/extensions/api/PRESUBMIT_test.py:44: 'Expecting , delimiter: line
8 column 12 (char 18)',
Here I find a strange problem: It looks like the error messages are different
from different OS.

On windows, the error messages should be
  'Expecting property name: line 8 column 3 (char 16)',
  'Invalid control character at: line 8 column 19 (char 32)',
  'Expecting property name: line 8 column 23 (char 36)',
  'Expecting , delimiter: line 8 column 12 (char 25)',
   
While on Linux, the error messages should be
  'Expecting property name: line 8 column 3 (char 9)',
  'Invalid control character at: line 8 column 19 (char 25)',
  'Expecting property name: line 8 column 23 (char 29)',
  'Expecting , delimiter: line 8 column 12 (char 18)',

The last word(char XX) is different. From buildbot stdio log, it seems using
linux presubmit.

[not at google - send to devlin, 2013-09-18 15:30:06]

On 2013/09/18 02:00:37, Haojian Wu wrote:
>
https://codereview.chromium.org/23512007/diff/76001/chrome/common/extensions/...
> File chrome/common/extensions/api/PRESUBMIT_test.py (right):
> 
>
https://codereview.chromium.org/23512007/diff/76001/chrome/common/extensions/...
> chrome/common/extensions/api/PRESUBMIT_test.py:44: 'Expecting , delimiter:
line
> 8 column 12 (char 18)',
> Here I find a strange problem: It looks like the error messages are different
> from different OS.
> 
> On windows, the error messages should be
>   'Expecting property name: line 8 column 3 (char 16)',
>   'Invalid control character at: line 8 column 19 (char 32)',
>   'Expecting property name: line 8 column 23 (char 36)',
>   'Expecting , delimiter: line 8 column 12 (char 25)',
>    
> While on Linux, the error messages should be
>   'Expecting property name: line 8 column 3 (char 9)',
>   'Invalid control character at: line 8 column 19 (char 25)',
>   'Expecting property name: line 8 column 23 (char 29)',
>   'Expecting , delimiter: line 8 column 12 (char 18)',
> 
> The last word(char XX) is different. From buildbot stdio log, it seems using
> linux presubmit.

Maybe it's that the windows has \r characters and includes those? Perhaps try
removing \r characters after reading the file for the test?

[Haojian Wu, 2013-09-22 16:32:59]

On 2013/09/18 15:30:06, kalman wrote:
> Maybe it's that the windows has \r characters and includes those? Perhaps try
> removing \r characters after reading the file for the test?

Thanks for the hints.

I have figured out the reason. It causes by subprocess.Popen function. By
default universal_newlines is false, the stdout will be opened as text files
which makes lines are terminated by '\n' on linux, '\r' on mac and '\r\n' on
windows.

We should set flag universal_newlines in subprocess.Popen function, which makes
lines are terminated by '\n' on all platforms.

[Yoyo Zhou, 2013-09-22 18:17:21]

LGTM

https://codereview.chromium.org/23512007/diff/81001/chrome/common/extensions/...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://codereview.chromium.org/23512007/diff/81001/chrome/common/extensions/...
chrome/common/extensions/api/PRESUBMIT.py:29: process =
input_api.subprocess.Popen(
Do we need universal_newlines here too?

[commit-bot: I haz the power, 2013-09-22 23:08:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/81001

[Haojian Wu, 2013-09-23 04:21:42]

https://codereview.chromium.org/23512007/diff/81001/chrome/common/extensions/...
File chrome/common/extensions/api/PRESUBMIT.py (right):

https://codereview.chromium.org/23512007/diff/81001/chrome/common/extensions/...
chrome/common/extensions/api/PRESUBMIT.py:29: process =
input_api.subprocess.Popen(
On 2013/09/22 18:17:22, Yoyo Zhou wrote:
> Do we need universal_newlines here too?

Done.

[commit-bot: I haz the power, 2013-09-23 04:22:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/108001

[commit-bot: I haz the power, 2013-09-23 10:43:07]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-09-23 11:16:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/23512007/108001

[commit-bot: I haz the power, 2013-09-23 12:16:30]

Change committed as 224709