Hi Michael,
I already answered you in Jira here it is another more complete version:
Removing trailing spaces has been discussed in the past and the consensus is that we don't remove trailing spaces in patches because it hurts
reviewers with false changes.
What was suggested then is to split formatting changes from functional changes when creating patches, here is an example of this suggestion[1]. At
least we could split trailing spaces removing patches they can sometimes be really a pain! As I said in the Jira another solution would be to use a
reviewing tool. I tried https://reviews.apache.org in the past[0] but was finally not satisfied and got back to my email client.
So, I'm reviewing directly in my email clients, and IDE is not relevant for me. Maybe a solution would be to suggest another consensual reviewing
process, there are tools for that[2]. And then possibly removing trailing spaces would be a good thing indeed.
This is a bit out of subject, but while at it:
As you can see at [3] I'm not against removing trailing spaces. But experience told me one thing since. One of the most important things in version
control is what happened to a line, because you will need to track issues in the past. This with svn (harder but possible with Git) is done with
annotations (aka blame). When we make batch changes (like I had to do for definitely resolving the EOLs issue in SVN) you lose this information and
it's unfortunate. That's another good reason to not mix formatting changes with functional changes.
Jacques
[0] http://markmail.org/message/mdiokme77l3v2sja
[1] http://markmail.org/message/lbwz5puicmaleb7s
[2] https://en.wikipedia.org/wiki/List_of_tools_for_code_review
[3] http://markmail.org/message/ir7cs2ofbzbzc53a
Le 12/12/2017 à 10:27, Michael Brohl a écrit :
Hi Jacques,
I propose to do it the other way around: when reviewing diffs, you can configure the IDE to ignore these white space changes. It works perfectly in
Eclipse and you will not be bothered by such changes.
In my view, trailing white spaces do not belong there and should be removed as
part of the refactoring process. Else they would be there forever.
To be precise, these are not false changes but intended changes to get rid of
trailing white spaces.
Thanks,
Michael
Am 12.12.17 um 10:04 schrieb Jacques Le Roux (JIRA):
[
https://issues.apache.org/jira/browse/OFBIZ-9777?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16287290#comment-16287290
]
Jacques Le Roux commented on OFBIZ-9777:
----------------------------------------
hanks Michael, Julian,
Please Julian adjust your IDE or similar to not remove trailing white spaces when you create a patch, especially a big one, you may do that
temporarily. Consider that else reviewers have to be confronted with a lot of false changes, thanks!
BTW I have added a warning in
https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+Contributors+Best+Practices
for that
[FB] Package org.apache.ofbiz.product.imagemanagement
-----------------------------------------------------
Key: OFBIZ-9777
URL: https://issues.apache.org/jira/browse/OFBIZ-9777
Project: OFBiz
Issue Type: Sub-task
Components: product
Affects Versions: Trunk
Reporter: Julian Leichert
Assignee: Michael Brohl
Priority: Minor
Fix For: Upcoming Release
Attachments:
OFBIZ-9777_org.apache.ofbiz.product.imagemanagement_bugfixes.patch
