Page MenuHomePhabricator

Enable hook to block addition of trailing whitespace
Closed, DeclinedPublic

Description

I believe Tim said there was a default hook example for this.

Can we add it and make people clean up their additions?

Thanks!


Version: unspecified
Severity: enhancement

Details

Reference
bz35600

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 12:13 AM
bzimport added a project: Gerrit.
bzimport set Reference to bz35600.
bzimport added a subscriber: Unknown Object (MLST).

Right now, there's no update hook in gerrit yet[00], so we can't block them like we did with pre-commit in svn. There's a huge performance concern with it, so even if[01] is merged, I don't know if we can use it.

There's two workarounds we can do here:

  1. Make patchset-created complain about whitespace and -2 a commit if it's bad (there's examples for this in gerrit.git/contrib)
  2. Tweak the downloaded pre-commit hook to strip trailing whitespace (there's dozens of examples of how we could adjust pre-commit here)

[00] http://code.google.com/p/gerrit/issues/detail?id=925
[01] https://gerrit-review.googlesource.com/#/c/31350/

  • Bug 36169 has been marked as a duplicate of this bug. ***

Actually, if https://gerrit-review.googlesource.com/#/c/35231/ makes it into master, this may actually be possible. I'll have to keep an eye on it.

The more I think about this, I think it's a bad idea.

Trailing whitespace is completely harmless, and we should never programmatically yell at people for it.

Blocking commits due to something as silly as this will just discourage people from contributing.

If you hate trailing whitespace, there's plenty of ways to ignore it in diffs (both in Gerrit and locally).