Le 12/12/2017 à 14:13, Nicolas Malin a écrit :
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
Ah, so I'm not alone, even if it's not the same tools ;)
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 :)
This was what David taught us and I found it reasonable
Thanks Nicolas
Jacques
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
