Hello
Le 12/12/2017 à 13:29, Michael Brohl a écrit :
[...] I have a very simple but effective workflow for this, you might
find it helpful also:
1. copy the link to the patch file in the Jira issue
2. right click on your OFBiz project in Eclipse and select Team/Apply
patch
3. the copied link should be filled in the Url selection, just click next
4. select the project to apply the patch. It should be pre-selected so
just click next
You now get a nice view with all changes and a graphical diff to
review the changes. If there are conflicts (e.g. the patch is
outdated), you can resolve them and change the patch on the fly.
Oh, it's not easier for all, like me, too long, I use directly emacs
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.
Of course it's a little difficult when you are just looking at plain
text. You can use a trick: just mark the diff part where you are
searching for a change and you'll see the trailing whitespace.
Personally, I cannot imagine to review patches in an email client
without having a look at the context (code around the diffs). This
might work for very simple things but definitely not for more
comprehensive patches.
Damned, I'm an young old man ? because I review 9 patch / 10 directly on
the email content, If I have a doubt, switch to emacs and if I no
understand more switch to intellij
I catch up to Jacques's comment, I prefer to separate the formating to
the functional even if I was do the same in the past and I will do it by
oversight in the future :)
Nicolas
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