nrwt: remove --no-record-results
Created attachment 177618 [details] Patch
Comment on attachment 177618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177618&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:376 > + num_workers=1, retrying=True) nit: Don't we typically only indent things like this 4 spaces?
Comment on attachment 177618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177618&action=review >> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:376 >> + num_workers=1, retrying=True) > > nit: Don't we typically only indent things like this 4 spaces? I think it's acceptable to either do 4 spaces or line up w/ the opening paren.
(In reply to comment #3) > (From update of attachment 177618 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177618&action=review > > >> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:376 > >> + num_workers=1, retrying=True) > > > > nit: Don't we typically only indent things like this 4 spaces? > > I think it's acceptable to either do 4 spaces or line up w/ the opening paren. No, WebKit style is to always indent by 4 spaces.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 177618 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=177618&action=review > > > > >> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:376 > > >> + num_workers=1, retrying=True) > > > > > > nit: Don't we typically only indent things like this 4 spaces? > > > > I think it's acceptable to either do 4 spaces or line up w/ the opening paren. > > No, WebKit style is to always indent by 4 spaces. Hm. PEP-8 style is to line up with the paren, and we generally follow PEP-8 in Python code, as noted in http://trac.webkit.org/wiki/PythonGuidelines . It doesn't look like the main coding style web page ( http://www.webkit.org/coding/coding-style.html ) actually says anything about multi-line function calls one way or another, for any language. And, I would bet that more python code follows PEP-8 in this regard than WK style. That said, I don't really care. Looks like both pep8 and pylint don't complain about this either way. I will fix this case.
Committed r136768: <http://trac.webkit.org/changeset/136768>
(In reply to comment #5) > Hm. PEP-8 style is to line up with the paren, and we generally follow PEP-8 in Python code, as noted in http://trac.webkit.org/wiki/PythonGuidelines . It doesn't look like the main coding style web page ( http://www.webkit.org/coding/coding-style.html ) actually says anything about multi-line function calls one way or another, for any language. And, I would bet that more python code follows PEP-8 in this regard than WK style. "The indent size is 4 spaces." in the indentation section refers to ALL indentations.
(In reply to comment #7) > (In reply to comment #5) > > Hm. PEP-8 style is to line up with the paren, and we generally follow PEP-8 in Python code, as noted in http://trac.webkit.org/wiki/PythonGuidelines . It doesn't look like the main coding style web page ( http://www.webkit.org/coding/coding-style.html ) actually says anything about multi-line function calls one way or another, for any language. And, I would bet that more python code follows PEP-8 in this regard than WK style. > > "The indent size is 4 spaces." in the indentation section refers to ALL indentations. If you say so. It would be nice if it mentioned this case explicitly, because I would not have (and didn't) make that leap. Also, it's not obvious that that page does (or should) apply to python code.
(In reply to comment #8) > If you say so. It would be nice if it mentioned this case explicitly, because I would not have (and didn't) make that leap. Feel free to submit a patch but this had never come up on C++ side of things. I think the part of confusion comes from old python code that followed Google style guide instead.
(In reply to comment #9) > (In reply to comment #8) > > If you say so. It would be nice if it mentioned this case explicitly, because I would not have (and didn't) make that leap. > > Feel free to submit a patch but this had never come up on C++ side of things. I think the part of confusion comes from old python code that followed Google style guide instead. No, this is coming up because I write code that follows PEP-8 style even for new code :).
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > If you say so. It would be nice if it mentioned this case explicitly, because I would not have (and didn't) make that leap. > > > > Feel free to submit a patch but this had never come up on C++ side of things. I think the part of confusion comes from old python code that followed Google style guide instead. > > No, this is coming up because I write code that follows PEP-8 style even for new code :). But PEP8 doesn’t prevent indentations to be always 4 spaces.
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > (In reply to comment #8) > > > > If you say so. It would be nice if it mentioned this case explicitly, because I would not have (and didn't) make that leap. > > > > > > Feel free to submit a patch but this had never come up on C++ side of things. I think the part of confusion comes from old python code that followed Google style guide instead. > > > > No, this is coming up because I write code that follows PEP-8 style even for new code :). > > But PEP8 doesn’t prevent indentations to be always 4 spaces. PEP-8 (the actual standard) says that function arguments should be aligned with the paren, and that left-indenting is wrong. pep8.py (the tool) and pylint do not seem to enforce that. http://www.python.org/dev/peps/pep-0008/#indentation
(In reply to comment #12) > (In reply to comment #11) > > But PEP8 doesn’t prevent indentations to be always 4 spaces. > > PEP-8 (the actual standard) says that function arguments should be aligned with the paren, and that left-indenting is wrong. pep8.py (the tool) and pylint do not seem to enforce that. > > http://www.python.org/dev/peps/pep-0008/#indentation See examples: # More indentation included to distinguish this from the rest. def long_function_name( var_one, var_two, var_three, var_four): print(var_one) and # Extra indentation is not necessary. foo = long_function_name( var_one, var_two, var_three, var_four) are both acceptable.
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > But PEP8 doesn’t prevent indentations to be always 4 spaces. > > > > PEP-8 (the actual standard) says that function arguments should be aligned with the paren, and that left-indenting is wrong. pep8.py (the tool) and pylint do not seem to enforce that. > > > > http://www.python.org/dev/peps/pep-0008/#indentation > > See examples: > # More indentation included to distinguish this from the rest. > def long_function_name( > var_one, var_two, var_three, > var_four): > print(var_one) > > and > > # Extra indentation is not necessary. > foo = long_function_name( > var_one, var_two, > var_three, var_four) > > are both acceptable. Okay, there are some cases where you not aligning w/ the left paren appear to be okay, but the line ojan commented on has variables on the first line and the second (and only) subsequent line: it matches the first "no" example, not the second "yes" example or the "optional" example.