FAQ
I propose addition to QA bot - check for trailing whitespace in patches.
Probably checking for tabs in Java files would be also good idea.

Search Discussions

  • Harsh J at Nov 25, 2012 at 7:35 pm
    IIRC we still have a checkstyle section in the QA bot's checking
    script, if that still is interesting for devs to turn on again. At
    some point it was probably disabled.
    On Mon, Nov 26, 2012 at 12:44 AM, Radim Kolar wrote:
    I propose addition to QA bot - check for trailing whitespace in patches.
    Probably checking for tabs in Java files would be also good idea.


    --
    Harsh J
  • Aaron T. Myers at Nov 26, 2012 at 5:45 pm

    On Sun, Nov 25, 2012 at 11:14 AM, Radim Kolar wrote:

    I propose addition to QA bot - check for trailing whitespace in patches.
    Probably checking for tabs in Java files would be also good idea.
    Checking for tabs I could get behind; checking for trailing whitespace not
    so much.

    I've never understood why folks get worked up over a little trailing
    whitespace here and there, since you can't see it and it doesn't affect
    correctness. Spurious whitespace changes that make a review harder - those
    are annoying. Trailing whitespace inadvertently left on lines where
    legitimate changes were made in a patch - doesn't seem too harmful to me.

    --
    Aaron T. Myers
    Software Engineer, Cloudera
  • Colin McCabe at Nov 26, 2012 at 6:00 pm
    Some projects have a subversion pre or post-commit hook to check for
    and/or correct these kinds of issues.

    Inconsistent line endings (CRLF/LF), bogus tabs, and extra whitespace
    are all things we could check for and/or correct this way.

    Colin

    On Mon, Nov 26, 2012 at 9:44 AM, Aaron T. Myers wrote:
    On Sun, Nov 25, 2012 at 11:14 AM, Radim Kolar wrote:

    I propose addition to QA bot - check for trailing whitespace in patches.
    Probably checking for tabs in Java files would be also good idea.
    Checking for tabs I could get behind; checking for trailing whitespace not
    so much.

    I've never understood why folks get worked up over a little trailing
    whitespace here and there, since you can't see it and it doesn't affect
    correctness. Spurious whitespace changes that make a review harder - those
    are annoying. Trailing whitespace inadvertently left on lines where
    legitimate changes were made in a patch - doesn't seem too harmful to me.

    --
    Aaron T. Myers
    Software Engineer, Cloudera
  • Radim Kolar at Nov 26, 2012 at 6:55 pm

    I've never understood why folks get worked up over a little trailing
    whitespace here and there, since you can't see it and it doesn't affect
    correctness. Spurious whitespace changes that make a review harder - those
    are annoying. Trailing whitespace inadvertently left on lines where
    legitimate changes were made in a patch - doesn't seem too harmful to me.
    Trailing whitespace is annoying because:
    if you have editor to set killing it, it will produce large patch.
    if use use scroll up at end of line, then cursor will not jump to
    end of text but some space after it, it cost you more clicks for cursor
    movement and it is annoying if it ends of split line.
    its good and standard practise to avoid it, git and other tools
    highlight it in red.
    if you use ignore whitespace in git diff, it often produces patch
    failing to apply

    Trailing whitespace can be striped by pre-commit hook.
  • Aaron T. Myers at Nov 26, 2012 at 11:54 pm
    OK, if folks want to do something to get rid of trailing whitespace in the
    project I won't object, but it doesn't seem like that big a deal to me. A
    pre-commit hook makes sense to me. I just don't want to see the QA bot flag
    patches containing trailing whitespace, thus requiring more round trips on
    patches.

    --
    Aaron T. Myers
    Software Engineer, Cloudera


    On Mon, Nov 26, 2012 at 10:53 AM, Radim Kolar wrote:

    I've never understood why folks get worked up over a little trailing
    whitespace here and there, since you can't see it and it doesn't affect
    correctness. Spurious whitespace changes that make a review harder - those
    are annoying. Trailing whitespace inadvertently left on lines where
    legitimate changes were made in a patch - doesn't seem too harmful to me.
    Trailing whitespace is annoying because:
    if you have editor to set killing it, it will produce large patch.
    if use use scroll up at end of line, then cursor will not jump to end
    of text but some space after it, it cost you more clicks for cursor
    movement and it is annoying if it ends of split line.
    its good and standard practise to avoid it, git and other tools
    highlight it in red.
    if you use ignore whitespace in git diff, it often produces patch
    failing to apply

    Trailing whitespace can be striped by pre-commit hook.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupcommon-dev @
categorieshadoop
postedNov 25, '12 at 7:15p
activeNov 26, '12 at 11:54p
posts6
users4
websitehadoop.apache.org...
irc#hadoop

People

Translate

site design / logo © 2021 Grokbase