Last Comment Bug 855712 - [CODE CLEANUP] trailing whitespaces in image/src/*cpp
: [CODE CLEANUP] trailing whitespaces in image/src/*cpp
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 Linux
: P5 normal (vote)
: mozilla22
Assigned To: Luis de Bethencourt [:luisbg]
:
: Milan Sreckovic [:milan]
Mentors:
https://developer.mozilla.org/en-US/d...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-28 08:19 PDT by Luis de Bethencourt [:luisbg]
Modified: 2013-03-30 17:55 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
trailing whitespaces removed (18.76 KB, patch)
2013-03-28 08:20 PDT, Luis de Bethencourt [:luisbg]
jmuizelaar: review+
Details | Diff | Splinter Review
rebased against current trunk and hg formatted (18.90 KB, patch)
2013-03-29 08:45 PDT, Luis de Bethencourt [:luisbg]
luis: checkin+
Details | Diff | Splinter Review
rebased and updated (25.50 KB, patch)
2013-03-29 12:23 PDT, Luis de Bethencourt [:luisbg]
no flags Details | Diff | Splinter Review

Description Luis de Bethencourt [:luisbg] 2013-03-28 08:19:49 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.69 Safari/537.17

Steps to reproduce:

Reading the Graphics code in image/src/ and noticed a lot of trailing whitespaces. My text editor automatically offers me to remove them so decided it is worth sharing and cleaning up.

If there is interest I could clean the code to follow https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style
Comment 1 Luis de Bethencourt [:luisbg] 2013-03-28 08:20:41 PDT
Created attachment 730684 [details] [diff] [review]
trailing whitespaces removed
Comment 2 Jonathan Kew (:jfkthame) 2013-03-28 09:14:00 PDT
Comment on attachment 730684 [details] [diff] [review]
trailing whitespaces removed

You shouldn't be setting "review+" on your patch - you need to set "review?" (with an appropriate email address) as a request for a Graphics module owner/peer to review the patch, then they'll change it to "+" or "-" as appropriate.

In this case, I think the main question is whether the cleanup is worth the pollution of history/hg blame that it causes.
Comment 3 Luis de Bethencourt [:luisbg] 2013-03-28 09:40:43 PDT
Jonathan,

Thanks for feedback. I agree that polluting the hg blame is the negative aspect of this cleanup. I wish there was a way to do this kind of cleanups without changing the history/blame of these lines.

I will find somebody else in the Graphics team to assign the review to since Joe is in vacations.
Comment 4 Jeff Muizelaar [:jrmuizel] 2013-03-29 08:22:07 PDT
Comment on attachment 730684 [details] [diff] [review]
trailing whitespaces removed

Review of attachment 730684 [details] [diff] [review]:
-----------------------------------------------------------------

This is certainly reasonable. Hopefully it doesn't cause Joe any unnecessary pain with bug 716140
Comment 5 Luis de Bethencourt [:luisbg] 2013-03-29 08:45:23 PDT
Created attachment 731176 [details] [diff] [review]
rebased against current trunk and hg formatted

Thanks Jeff for the r+ :)
Comment 6 Luis de Bethencourt [:luisbg] 2013-03-29 12:23:24 PDT
Created attachment 731262 [details] [diff] [review]
rebased and updated

Updated
Comment 7 Luis de Bethencourt [:luisbg] 2013-03-29 12:30:14 PDT
Comment on attachment 731262 [details] [diff] [review]
rebased and updated

Review of attachment 731262 [details] [diff] [review]:
-----------------------------------------------------------------

setting it as checkin:?
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-03-29 13:14:33 PDT
Comment on attachment 731262 [details] [diff] [review]
rebased and updated

You can just set the checkin-needed keyword at the top in the future. It's a bit easier for me to use.
Comment 9 Ryan VanderMeulen [:RyanVM] 2013-03-29 13:16:49 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc53af70fd58
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-03-30 17:55:06 PDT
https://hg.mozilla.org/mozilla-central/rev/cc53af70fd58

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