Last modified: 2013-02-19 18:21:53 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T37600, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 35600 - Enable hook to block addition of trailing whitespace
Enable hook to block addition of trailing whitespace
Status: RESOLVED WONTFIX
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Unprioritized enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-29 23:37 UTC by Sam Reed (reedy)
Modified: 2013-02-19 18:21 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Sam Reed (reedy) 2012-03-29 23:37:22 UTC
I believe Tim said there was a default hook example for this.

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


Thanks!
Comment 1 Chad H. 2012-04-12 21:41:01 UTC
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/
Comment 2 Chad H. 2012-04-23 19:13:41 UTC
*** Bug 36169 has been marked as a duplicate of this bug. ***
Comment 3 Chad H. 2012-07-28 01:03:06 UTC
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.
Comment 4 Chad H. 2013-02-19 18:21:53 UTC
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).

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links