Last modified: 2014-01-31 10:56:35 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 T38169, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 36169 - Whitespace fixer to gerrit
Whitespace fixer to gerrit
Status: REOPENED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Low enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: upstream
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-23 10:52 UTC by Niklas Laxström
Modified: 2014-01-31 10:56 UTC (History)
7 users (show)

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


Attachments

Description Niklas Laxström 2012-04-23 10:52:04 UTC
I would be very handy if we could edit code directly in gerrit to fix some small issues like whitespace. Or have a button that removes trailing whitespace and submits a new patchset.

Another way I guess is to prevent submission of patches which contain trailing whitespace.
Comment 1 Chad H. 2012-04-23 18:19:04 UTC
(In reply to comment #0)
> I would be very handy if we could edit code directly in gerrit to fix some
> small issues like whitespace. Or have a button that removes trailing whitespace
> and submits a new patchset.
> 

That might be doable.

> Another way I guess is to prevent submission of patches which contain trailing
> whitespace.

pre-receive hooks are not possible in Gerrit, see issue 925 (http://code.google.com/p/gerrit/issues/detail?id=925)
Comment 2 Chad H. 2012-04-23 19:13:41 UTC
Actually, this is a dupe.

*** This bug has been marked as a duplicate of bug 35600 ***
Comment 3 Niklas Laxström 2012-04-24 08:42:47 UTC
It is only dupe if you consider my alternative suggestion. But like said that path doesn't look likely, so I'm reopening this so that we can consider other solutions.
Comment 4 Krinkle 2012-04-27 10:01:10 UTC
Since this bug is primary summarized as fixing whitespace and not an inline editor per se (that could be another feature request), I'll reply as such.

In `git diff` (or after `git add`, in `git diff --cached`) whitespace is highlighted in bright red (depending on your settings, I use ".gitconfig: [color] diff = auto"). So one should check that before committing as part of the general pre-commit check.

Then one can fix that in whatever way you prefer (open up the file in an editor and fix it by hand, or use a find/replace function and replace " \n" and "\t\n" with "\n"), or do it with some utility from the command line (we could add a script to /tools that checks which files have been modified and fix the whitespace of those). But in general those kind of changes should be made in separate commits.
Comment 5 Niklas Laxström 2012-04-27 11:31:18 UTC
That is not the point. The round-trip of downloading the change (cloning the repo first if you don't have it) and submitting it again is excessive for small changes and delays the merge to master.
Comment 6 Antoine "hashar" Musso (WMF) 2012-06-28 13:04:37 UTC
we could use `git diff --check` as a build step in Jenkins. That would directly reject such a change.
Comment 7 Chad H. 2013-01-09 15:27:23 UTC
Doing this would be nice, and there's some mumblings of adding support for CodeMirror[0] for doing this sort of inline editing. No promises, I just heard some "hey that looks cool, wonder if we could..."

[0] http://codemirror.net/
Comment 8 Andre Klapper 2013-02-19 17:48:30 UTC
Slightly related to https://code.google.com/p/gerrit/issues/detail?id=505 ("Allow reviewer to make code changes"), though this request is not limited to the reviewer.

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


Navigation
Links