Closed Bug 855712 Opened 7 years ago Closed 7 years ago

[CODE CLEANUP] trailing whitespaces in image/src/*cpp

Categories

(Core :: Graphics, defect, P5)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: luis, Assigned: luis)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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
Attached patch trailing whitespaces removed (obsolete) — Splinter Review
Attachment #730684 - Flags: review+
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.
Attachment #730684 - Flags: review+ → review?(joe)
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.
Attachment #730684 - Flags: review?(joe) → review?(bobbyholley+bmo)
Attachment #730684 - Flags: review?(bobbyholley+bmo) → review?(jmuizelaar)
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
Attachment #730684 - Flags: review?(jmuizelaar) → review+
Thanks Jeff for the r+ :)
Attachment #731176 - Flags: checkin+
Updated
Attachment #730684 - Attachment is obsolete: true
Attachment #731176 - Attachment is obsolete: true
Attachment #731262 - Flags: checkin+
Comment on attachment 731262 [details] [diff] [review]
rebased and updated

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

setting it as checkin:?
Attachment #731262 - Flags: checkin+ → checkin?
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.
Attachment #731262 - Flags: checkin?
https://hg.mozilla.org/mozilla-central/rev/cc53af70fd58
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.