Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacopo Cappellato
I was talking about consistency from now on; I was not suggesting to bulk
change everything.

Jacopo

On Tue, Sep 13, 2016 at 8:40 AM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Hi,
>
> Wait, I did not ask to remove them. I simply asked "Should we continue to
> put them, and if yes for which reasons? "
>
> I don't want to remove existing ones (easy done in one shoot with a S/R
> regexp) because it would remove the precious svn annotations (when you want
> to look for reasons in the past)
>
> I just want to stop put them when we refactor/fix/etc.
>
> I know it will introduce inconsistency (and I assume you know how much I
> dislike inconsistency) but if the price to pay is to lose again the
> *precious svn annotation*, by changing almost of lines of all Groovy files,
> then I'd say it's not worth it
>
> Thank you Rishi for the link to https://cwiki.apache.org/confl
> uence/display/OFBIZ/Tips+and+Tricks+while+working+with+Groovy I did not
> remember of it. I changed the title to make the URL easier to read
>
> Jacques
>
>
>
> Le 13/09/2016 à 08:23, Rishi Solanki a écrit :
>
>> Jacopo,
>>
>> I could recall after your reply, the semicolon kept in the groovy files
>> for
>> consistency only no other reason. Also, if we decide to remove it, then
>> only thing we should consider if somewhere semicolon used as separator.
>>
>> Thanks!
>>
>> --
>> Rishi
>>
>> Rishi Solanki
>> Manager, Enterprise Software Development
>> HotWax Systems Pvt. Ltd.
>> Direct: +91-9893287847
>> http://www.hotwaxsystems.com
>>
>> On Tue, Sep 13, 2016 at 11:46 AM, Jacopo Cappellato <
>> jacopo.cappell...@hotwaxsystems.com> wrote:
>>
>> I don't have a strong opinion. But it would be nice to agreed upon one
>>> style and then implement consistently.
>>>
>>> Jacopo
>>>
>>> On Mon, Sep 12, 2016 at 6:56 PM, Jacques Le Roux <
>>> jacques.le.r...@les7arts.com> wrote:
>>>
>>> Hi

 While committing r1760406  I wondered if I should really put semicolons

>>> at
>>>
 end of Groovy files lines.
 We know it's useless in Groovy. Should we continue to put them, and if

>>> yes
>>>
 for which reasons?

 Thanks

 Jacques



>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 08:40, Jacques Le Roux a écrit :
Thank you Rishi for the link to https://cwiki.apache.org/confluence/display/OFBIZ/Tips+and+Tricks+while+working+with+Groovy I did not remember of 
it. I changed the title to make the URL easier to read


Jacques 


BTW the Beanshell references should be removed now. I suggest when doing so to put a link in the modified page to previous page versions in history 
and explain that to users in the link (it still true in releases and our users use releases).

Maybe not a bid deal in this case, rather a global way of doing changes in wiki 
for important matters

Thanks

Jacques



Re: Ideas about OFBiz servlet filters

2016-09-13 Thread Jacopo Cappellato
Thank you Jinghai,

I agree we should separate the concerns and split the ContextFilter into
two filters; I am going to work on this and I am planning to separate the
"controller" related concerns (like allowPaths and redirectPath functions)
into a new filter named ControlFilter.
But, shouldn't the ControlFilter be executed before the ContextFilter? Will
it conflict with the behavior of the CatalogUrlFilter and other filters?

Jacopo

On Sun, Sep 11, 2016 at 8:13 AM, Shi Jinghai  wrote:

> Great, Jacopo!
>
> I think it would be better to separate the allowPaths and redirectPath
> functions to a new filter. If ContextFilter be the 1st filter, the new
> filter will be the last I guess. Between them, CatalogUrlFilter and etc.
> will be there.
>
> Kind Regards,
>
> Shi Jinghai
>
> -邮件原件-
> 发件人: Jacopo Cappellato [mailto:jacopo.cappell...@hotwaxsystems.com]
> 发送时间: 2016年9月9日 16:07
> 收件人: dev@ofbiz.apache.org
> 主题: Ideas about OFBiz servlet filters
>
> A web application, in order to leverage the OFBiz framework, requires that
> a series of objects are in its contexts (servlet context, session and
> request) such as "delegator", "delegatorName", "dispatcher", "security"
> etc. etc...
> This setup is performed by the logic contained in the servlet filter
> implemented by the following class:
>
> org.apache.ofbiz.webapp.control.ContextFilter
>
> The execution of this logic is required for the application to run
> properly.
> However, this filter is deployed in most but not all the web application
> in the OFBiz codebase: there are few exceptions due to the fact that a few
> web applications require the execution of other filters (e.g.
> CatalogUrlFilter, etc...).
>
> Unfortunately the way these filters have been implemented have issues
> including:
> * some of them extend the ContextFilter and override its behavior by
> copying some logic and adding new one; in these cases the ContextFilter is
> also deployed but after the execution of the extended filter
> * some of them have been copied from ContextFilter and then adapted,
> introducing a lot of redundant code difficult to maintain; in these cases
> the ContextFilter is not deployed
>
> There is now a chance for the community to help cleaning up these classes
> and I am proposing the following guidelines:
>
> 1) servlet filters should be chained (rather than extended or replaced)
> 2) ContextFilter should always be used and should always be the first
> (OFBiz) filter in the chain
> 3) if some of the behavior/logic of ContextFilter conflicts with the ones
> of other filters, then ContextFilter should be enhanced to prevent that
> (e.g. we can improve the code, move some of its logic in a separate filter
> that can be executed after etc...)
> 4) the other filters should work well after the ContextFilter and add
> behavior rather than overriding behavior or duplicating behavior
>
> As a beneficial side effect of this effort, we will get a cleaner picture
> (documented by the logic of ContextFilter) of all the context objects
> required by OFBiz web applications.
>
> I hope it helps
>
> Jacopo
>


Limit the length of comments commits lines

2016-09-13 Thread Jacques Le Roux

Hi,

Is it possible to limit the length of comments commits lines? I saw a *split-lines* options but it seems to not work or at least I don't know how to 
use it and could not find any help.


Thanks

Jacques



Re: Ideas about OFBiz servlet filters

2016-09-13 Thread Shi Jinghai
Hi Jacopo,

Thanks for your consideration!

I like the name ControlFilter. On the sequence of the filters, personally I 
think it's a policy for entrance. If put the ControlFilter 1st, it's a strict 
control. If put it last, it's a loose control. Anyway, we need it.

When you complete, I will try to change SEO and solr filters to follow your 
update.

Kind Regards,

Shi Jinghai


-邮件原件-
发件人: Jacopo Cappellato [mailto:jacopo.cappell...@hotwaxsystems.com] 
发送时间: 2016年9月13日 16:30
收件人: dev@ofbiz.apache.org
主题: Re: Ideas about OFBiz servlet filters

Thank you Jinghai,

I agree we should separate the concerns and split the ContextFilter into two 
filters; I am going to work on this and I am planning to separate the 
"controller" related concerns (like allowPaths and redirectPath functions) into 
a new filter named ControlFilter.
But, shouldn't the ControlFilter be executed before the ContextFilter? Will it 
conflict with the behavior of the CatalogUrlFilter and other filters?

Jacopo

On Sun, Sep 11, 2016 at 8:13 AM, Shi Jinghai  wrote:

> Great, Jacopo!
>
> I think it would be better to separate the allowPaths and redirectPath 
> functions to a new filter. If ContextFilter be the 1st filter, the new 
> filter will be the last I guess. Between them, CatalogUrlFilter and etc.
> will be there.
>
> Kind Regards,
>
> Shi Jinghai
>
> -邮件原件-
> 发件人: Jacopo Cappellato [mailto:jacopo.cappell...@hotwaxsystems.com]
> 发送时间: 2016年9月9日 16:07
> 收件人: dev@ofbiz.apache.org
> 主题: Ideas about OFBiz servlet filters
>
> A web application, in order to leverage the OFBiz framework, requires 
> that a series of objects are in its contexts (servlet context, session 
> and
> request) such as "delegator", "delegatorName", "dispatcher", "security"
> etc. etc...
> This setup is performed by the logic contained in the servlet filter 
> implemented by the following class:
>
> org.apache.ofbiz.webapp.control.ContextFilter
>
> The execution of this logic is required for the application to run 
> properly.
> However, this filter is deployed in most but not all the web 
> application in the OFBiz codebase: there are few exceptions due to the 
> fact that a few web applications require the execution of other filters (e.g.
> CatalogUrlFilter, etc...).
>
> Unfortunately the way these filters have been implemented have issues
> including:
> * some of them extend the ContextFilter and override its behavior by 
> copying some logic and adding new one; in these cases the 
> ContextFilter is also deployed but after the execution of the extended 
> filter
> * some of them have been copied from ContextFilter and then adapted, 
> introducing a lot of redundant code difficult to maintain; in these 
> cases the ContextFilter is not deployed
>
> There is now a chance for the community to help cleaning up these 
> classes and I am proposing the following guidelines:
>
> 1) servlet filters should be chained (rather than extended or 
> replaced)
> 2) ContextFilter should always be used and should always be the first
> (OFBiz) filter in the chain
> 3) if some of the behavior/logic of ContextFilter conflicts with the 
> ones of other filters, then ContextFilter should be enhanced to 
> prevent that (e.g. we can improve the code, move some of its logic in 
> a separate filter that can be executed after etc...)
> 4) the other filters should work well after the ContextFilter and add 
> behavior rather than overriding behavior or duplicating behavior
>
> As a beneficial side effect of this effort, we will get a cleaner 
> picture (documented by the logic of ContextFilter) of all the context 
> objects required by OFBiz web applications.
>
> I hope it helps
>
> Jacopo
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Scott Gray
>
> I don't want to remove existing ones (easy done in one shoot with a S/R
> regexp) because it would remove the precious svn annotations (when you want
> to look for reasons in the past)


Hi Jacques,

What are these precious svn annotations used for?  Maybe I'm out of the
loop since I use git-svn which let's me search history a million different
ways, but I'm interested to know why the annotations would be problematic
for bulk S/R operations.

Thanks
Scott

On 13 September 2016 at 18:40, Jacques Le Roux  wrote:

> Hi,
>
> Wait, I did not ask to remove them. I simply asked "Should we continue to
> put them, and if yes for which reasons? "
>
> I don't want to remove existing ones (easy done in one shoot with a S/R
> regexp) because it would remove the precious svn annotations (when you want
> to look for reasons in the past)
>
> I just want to stop put them when we refactor/fix/etc.
>
> I know it will introduce inconsistency (and I assume you know how much I
> dislike inconsistency) but if the price to pay is to lose again the
> *precious svn annotation*, by changing almost of lines of all Groovy files,
> then I'd say it's not worth it
>
> Thank you Rishi for the link to https://cwiki.apache.org/confl
> uence/display/OFBIZ/Tips+and+Tricks+while+working+with+Groovy I did not
> remember of it. I changed the title to make the URL easier to read
>
> Jacques
>
>
>
> Le 13/09/2016 à 08:23, Rishi Solanki a écrit :
>
>> Jacopo,
>>
>> I could recall after your reply, the semicolon kept in the groovy files
>> for
>> consistency only no other reason. Also, if we decide to remove it, then
>> only thing we should consider if somewhere semicolon used as separator.
>>
>> Thanks!
>>
>> --
>> Rishi
>>
>> Rishi Solanki
>> Manager, Enterprise Software Development
>> HotWax Systems Pvt. Ltd.
>> Direct: +91-9893287847
>> http://www.hotwaxsystems.com
>>
>> On Tue, Sep 13, 2016 at 11:46 AM, Jacopo Cappellato <
>> jacopo.cappell...@hotwaxsystems.com> wrote:
>>
>> I don't have a strong opinion. But it would be nice to agreed upon one
>>> style and then implement consistently.
>>>
>>> Jacopo
>>>
>>> On Mon, Sep 12, 2016 at 6:56 PM, Jacques Le Roux <
>>> jacques.le.r...@les7arts.com> wrote:
>>>
>>> Hi

 While committing r1760406  I wondered if I should really put semicolons

>>> at
>>>
 end of Groovy files lines.
 We know it's useless in Groovy. Should we continue to put them, and if

>>> yes
>>>
 for which reasons?

 Thanks

 Jacques



>


Re: Limit the length of comments commits lines

2016-09-13 Thread Jacopo Cappellato
Jacques, do we all have to read your correspondence with TortoiseSvn? This
list is reserved for discussions related to development of OFBiz.
If you will get useful hints for OFBiz developers using your same tools you
could always put the information in the Wiki.

Jacopo

On Tue, Sep 13, 2016 at 10:58 AM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Hi,
>
> Is it possible to limit the length of comments commits lines? I saw a
> *split-lines* options but it seems to not work or at least I don't know how
> to use it and could not find any help.
>
> Thanks
>
> Jacques
>
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 11:34, Scott Gray a écrit :

I don't want to remove existing ones (easy done in one shoot with a S/R
regexp) because it would remove the precious svn annotations (when you want
to look for reasons in the past)


Hi Jacques,

What are these precious svn annotations used for?  Maybe I'm out of the
loop since I use git-svn which let's me search history a million different
ways, but I'm interested to know why the annotations would be problematic
for bulk S/R operations.

Thanks
Scott

Hi Scott,

Sometimes I try to understand why and when a line has been committed this is 
mostly when the annotations are useful to me.
Also sometimes simply to find a revision number.

HTH

Jacques


Re: Groovy and semicolon at EOL

2016-09-13 Thread Michael Brohl
Same here. I'm not even sure if we really have clean groovy in the 
project, I assume it is mixed up with Java code in some areas.


But I agree to have a consistent style and we should use the Groovy 
language as it shoul be used (even if I would have get used to it and  
like a a defined code line ending better).


I see the following directions:

1. actively migrate to pure groovy and remove the semicolons (where 
applicable, it seems there are some cases where you need them, see 
https://dzone.com/articles/groovy-sometimes-you-still)


2. activeley put semicolons everywhere for consistency

3. do 1., but only when a groovy file is edited anyway. This would 
slowly migrate groovy files.


I'd be in favor for 3., as long as there are other more important things 
to do or there is a volunteer to do it.


Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:

Okay I missed the historical context.

Like Jacopo I also do not have a strong opinion, if it is easier and faster
to keep them, then keep them. The important thing is to take a direction
and stay with it.







smime.p7s
Description: S/MIME Cryptographic Signature


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 11:56, Michael Brohl a écrit :

Same here. I'm not even sure if we really have clean groovy in the project, I 
assume it is mixed up with Java code in some areas.

But I agree to have a consistent style and we should use the Groovy language as it shoul be used (even if I would have get used to it and  like a a 
defined code line ending better).


I see the following directions:

1. actively migrate to pure groovy and remove the semicolons (where applicable, it seems there are some cases where you need them, see 
https://dzone.com/articles/groovy-sometimes-you-still)


2. activeley put semicolons everywhere for consistency

3. do 1., but only when a groovy file is edited anyway. This would slowly 
migrate groovy files.

I'd be in favor for 3., as long as there are other more important things to do 
or there is a volunteer to do it.


This is what I somehow suggested, thanks for clarifying Michael! Better to have 
consistent lines (with respect to semicolons) by file indeed.

Jacques



Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:

Okay I missed the historical context.

Like Jacopo I also do not have a strong opinion, if it is easier and faster
to keep them, then keep them. The important thing is to take a direction
and stay with it.










Re: Groovy and semicolon at EOL

2016-09-13 Thread Taher Alkhateeb
Yup +1 for option 3, fix as you edit

On Sep 13, 2016 1:16 PM, "Jacques Le Roux" 
wrote:

> Le 13/09/2016 à 11:56, Michael Brohl a écrit :
>
>> Same here. I'm not even sure if we really have clean groovy in the
>> project, I assume it is mixed up with Java code in some areas.
>>
>> But I agree to have a consistent style and we should use the Groovy
>> language as it shoul be used (even if I would have get used to it and  like
>> a a defined code line ending better).
>>
>> I see the following directions:
>>
>> 1. actively migrate to pure groovy and remove the semicolons (where
>> applicable, it seems there are some cases where you need them, see
>> https://dzone.com/articles/groovy-sometimes-you-still)
>>
>> 2. activeley put semicolons everywhere for consistency
>>
>> 3. do 1., but only when a groovy file is edited anyway. This would slowly
>> migrate groovy files.
>>
>> I'd be in favor for 3., as long as there are other more important things
>> to do or there is a volunteer to do it.
>>
>
> This is what I somehow suggested, thanks for clarifying Michael! Better to
> have consistent lines (with respect to semicolons) by file indeed.
>
> Jacques
>
>
>> Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:
>>
>>> Okay I missed the historical context.
>>>
>>> Like Jacopo I also do not have a strong opinion, if it is easier and
>>> faster
>>> to keep them, then keep them. The important thing is to take a direction
>>> and stay with it.
>>>
>>>
>>>
>>
>>
>


Re: Fixing the way OFBiz deal with character encoding of http requests

2016-09-13 Thread Jacopo Cappellato
Committed in rev. 1760528

Jacopo

On Fri, Sep 9, 2016 at 3:33 PM, Jacopo Cappellato <
jacopo.cappell...@hotwaxsystems.com> wrote:

> In ContextFilter, the character encoding (aka charset) of every http
> *request* object is set using the WebAppUtil.setCharacterEncoding(...)
> method (see its logic here [*]).
>
> It is wrong to override the character encoding if already specified by the
> http request: in fact it doesn't make any sense to try to decode an http
> request, whose body content is encoded for example with ISO-8859-1, using
> the OFBiz system's default of UTF-8.
>
> I propose instead to set the character encoding to the system default
> (e.g. UTF-8) if and only if it is not set already by the client.
>
> Any comments before I commit this change?
>
> Kind regards,
>
> Jacopo
>
> [*] the logic of WebAppUtil.setCharacterEncoding(...):
>
> public static void setCharacterEncoding(ServletRequest request) {
> String charset = request.getServletContext().getInitParameter("charset");
> if (UtilValidate.isEmpty(charset)) charset = 
> request.getCharacterEncoding();
> if (UtilValidate.isEmpty(charset)) charset = "UTF-8";
> if (!"none".equals(charset)) {
> request.setCharacterEncoding(charset);
> }
> }
>
>
>


buildbot failure in on ofbiz-trunk

2016-09-13 Thread buildbot
The Buildbot has detected a new failure on builder ofbiz-trunk while building . 
Full details are available at:
https://ci.apache.org/builders/ofbiz-trunk/builds/1415

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: orcus_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-ofbiz-commit' 
triggered this build
Build Source Stamp: [branch ofbiz/trunk] 1760528
Blamelist: jacopoc

BUILD FAILED: failed shell_1

Sincerely,
 -The Buildbot





Re: svn commit: r1760528 - /ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control /ContextFilter.java

2016-09-13 Thread Jacques Le Roux

You have got a small issue

https://ci.apache.org/builders/ofbiz-trunk/builds/1415

https://ci.apache.org/projects/ofbiz/logs/trunk/html/

Jacques


Le 13/09/2016 à 12:55, jaco...@apache.org a écrit :

Author: jacopoc
Date: Tue Sep 13 10:55:12 2016
New Revision: 1760528

URL: http://svn.apache.org/viewvc?rev=1760528&view=rev
Log:
Improved: set the character encoding to the system default (UTF-8) if and only
if it is not set already by the client.

Before this change the filter used to override the character encoding, even if
it was specified in the http request.
Also removed some unused imports and comments.


Modified:
 
ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java

Modified: 
ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java?rev=1760528&r1=1760527&r2=1760528&view=diff
==
--- 
ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
 (original)
+++ 
ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
 Tue Sep 13 10:55:12 2016
@@ -21,16 +21,13 @@ package org.apache.ofbiz.webapp.control;
  import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
  
  import java.io.IOException;

-import java.io.UnsupportedEncodingException;
  import java.util.Enumeration;
  import java.util.List;
  import java.util.Map;
-import java.util.Set;
  
  import javax.servlet.Filter;

  import javax.servlet.FilterChain;
  import javax.servlet.FilterConfig;
-import javax.servlet.ServletContext;
  import javax.servlet.ServletException;
  import javax.servlet.ServletRequest;
  import javax.servlet.ServletResponse;
@@ -50,12 +47,8 @@ import org.apache.ofbiz.entity.GenericVa
  import org.apache.ofbiz.entity.util.EntityQuery;
  import org.apache.ofbiz.entity.util.EntityUtil;
  import org.apache.ofbiz.security.Security;
-import org.apache.ofbiz.security.SecurityConfigurationException;
-import org.apache.ofbiz.security.SecurityFactory;
  import org.apache.ofbiz.service.LocalDispatcher;
-import org.apache.ofbiz.service.ServiceContainer;
  import org.apache.ofbiz.webapp.WebAppUtil;
-import org.apache.ofbiz.webapp.event.RequestBodyMapHandlerFactory;
  import org.apache.ofbiz.webapp.website.WebSiteWorker;
  
  /**

@@ -69,6 +62,9 @@ public class ContextFilter implements Fi
  protected FilterConfig config = null;
  protected boolean debug = false;
  
+// default charset used to decode requests body data if no encoding is specified in the request

+private String defaultCharacterEncoding;
+
  /**
   * @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
   */
@@ -84,6 +80,10 @@ public class ContextFilter implements Fi
  debug = Debug.verboseOn();
  }
  
+defaultCharacterEncoding = config.getServletContext().getInitParameter("charset");

+if (UtilValidate.isEmpty(defaultCharacterEncoding)) {
+defaultCharacterEncoding = "UTF-8";
+}
  // check the serverId
  getServerId();
  // initialize the delegator
@@ -104,7 +104,6 @@ public class ContextFilter implements Fi
  HttpServletRequest httpRequest = (HttpServletRequest) request;
  HttpServletResponse httpResponse = (HttpServletResponse) response;
  
-// Debug.logInfo("Running ContextFilter.doFilter", module);
  
  // - Servlet Object Setup -
  
@@ -250,10 +249,13 @@ public class ContextFilter implements Fi

  }
  }
  
+if (request.getCharacterEncoding() == null) {

+request.setCharacterEncoding(defaultCharacterEncoding);
+}
+WebAppUtil.setAttributesFromRequestBody(request);
+
  // check if multi tenant is enabled
  boolean useMultitenant = EntityUtil.isMultiTenantEnabled();
-WebAppUtil.setCharacterEncoding(request);
-WebAppUtil.setAttributesFromRequestBody(request);
  if (useMultitenant) {
  // get tenant delegator by domain name
  String serverName = httpRequest.getServerName();







Re: svn commit: r1760528 - /ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control /ContextFilter.java

2016-09-13 Thread Jacopo Cappellato
Yeah, thanks for the notification, I also saw the automatic build failure
email.
Weird, local tests are successful and that service doesn't seem to be
related to my last commit... but I am looking into it.

Jacopo

On Tue, Sep 13, 2016 at 1:15 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> You have got a small issue
>
> https://ci.apache.org/builders/ofbiz-trunk/builds/1415
>
> https://ci.apache.org/projects/ofbiz/logs/trunk/html/
>
> Jacques
>
>
>
> Le 13/09/2016 à 12:55, jaco...@apache.org a écrit :
>
>> Author: jacopoc
>> Date: Tue Sep 13 10:55:12 2016
>> New Revision: 1760528
>>
>> URL: http://svn.apache.org/viewvc?rev=1760528&view=rev
>> Log:
>> Improved: set the character encoding to the system default (UTF-8) if and
>> only
>> if it is not set already by the client.
>>
>> Before this change the filter used to override the character encoding,
>> even if
>> it was specified in the http request.
>> Also removed some unused imports and comments.
>>
>>
>> Modified:
>>  ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz
>> /webapp/control/ContextFilter.java
>>
>> Modified: ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
>> webapp/control/ContextFilter.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/sr
>> c/main/java/org/apache/ofbiz/webapp/control/ContextFilter.
>> java?rev=1760528&r1=1760527&r2=1760528&view=diff
>> 
>> ==
>> --- ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
>> webapp/control/ContextFilter.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
>> webapp/control/ContextFilter.java Tue Sep 13 10:55:12 2016
>> @@ -21,16 +21,13 @@ package org.apache.ofbiz.webapp.control;
>>   import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
>> import java.io.IOException;
>> -import java.io.UnsupportedEncodingException;
>>   import java.util.Enumeration;
>>   import java.util.List;
>>   import java.util.Map;
>> -import java.util.Set;
>> import javax.servlet.Filter;
>>   import javax.servlet.FilterChain;
>>   import javax.servlet.FilterConfig;
>> -import javax.servlet.ServletContext;
>>   import javax.servlet.ServletException;
>>   import javax.servlet.ServletRequest;
>>   import javax.servlet.ServletResponse;
>> @@ -50,12 +47,8 @@ import org.apache.ofbiz.entity.GenericVa
>>   import org.apache.ofbiz.entity.util.EntityQuery;
>>   import org.apache.ofbiz.entity.util.EntityUtil;
>>   import org.apache.ofbiz.security.Security;
>> -import org.apache.ofbiz.security.SecurityConfigurationException;
>> -import org.apache.ofbiz.security.SecurityFactory;
>>   import org.apache.ofbiz.service.LocalDispatcher;
>> -import org.apache.ofbiz.service.ServiceContainer;
>>   import org.apache.ofbiz.webapp.WebAppUtil;
>> -import org.apache.ofbiz.webapp.event.RequestBodyMapHandlerFactory;
>>   import org.apache.ofbiz.webapp.website.WebSiteWorker;
>> /**
>> @@ -69,6 +62,9 @@ public class ContextFilter implements Fi
>>   protected FilterConfig config = null;
>>   protected boolean debug = false;
>>   +// default charset used to decode requests body data if no
>> encoding is specified in the request
>> +private String defaultCharacterEncoding;
>> +
>>   /**
>>* @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
>>*/
>> @@ -84,6 +80,10 @@ public class ContextFilter implements Fi
>>   debug = Debug.verboseOn();
>>   }
>>   +defaultCharacterEncoding = config.getServletContext().get
>> InitParameter("charset");
>> +if (UtilValidate.isEmpty(defaultCharacterEncoding)) {
>> +defaultCharacterEncoding = "UTF-8";
>> +}
>>   // check the serverId
>>   getServerId();
>>   // initialize the delegator
>> @@ -104,7 +104,6 @@ public class ContextFilter implements Fi
>>   HttpServletRequest httpRequest = (HttpServletRequest) request;
>>   HttpServletResponse httpResponse = (HttpServletResponse)
>> response;
>>   -// Debug.logInfo("Running ContextFilter.doFilter", module);
>> // - Servlet Object Setup -
>>   @@ -250,10 +249,13 @@ public class ContextFilter implements Fi
>>   }
>>   }
>>   +if (request.getCharacterEncoding() == null) {
>> +request.setCharacterEncoding(defaultCharacterEncoding);
>> +}
>> +WebAppUtil.setAttributesFromRequestBody(request);
>> +
>>   // check if multi tenant is enabled
>>   boolean useMultitenant = EntityUtil.isMultiTenantEnabled();
>> -WebAppUtil.setCharacterEncoding(request);
>> -WebAppUtil.setAttributesFromRequestBody(request);
>>   if (useMultitenant) {
>>   // get tenant delegator by domain name
>>   String serverName = httpRequest.getServerName();
>>
>>
>>
>>
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Rishi Solanki
Fix as you edit, this is something like we are working on X functionality
and to achieve that functionality if we want to edit an groovy file, then
we will also remove/add semicolon to it.

If I'm understanding it correctly, then -1 for it. As we have to ask
explicitly to every contributor/committer to follow this practice on each
commit/ticket.

I'm up for #1 or #2 to actively remove/add semicolon. That is do it in one
shot, not immediately but whenever we are ready to do it, otherwise with
time we will have more inconsistency in groovy files on this parameter as
semicolon.

I'm not saying we must do it in one shot, but if community decides to
proceed with any approach to actively add/remove semicolon then we (@HW)
can try to assign single dev as volunteer to provide patch for all the
files.

Thanks!

Best Regards,
--

Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com

On Tue, Sep 13, 2016 at 3:49 PM, Taher Alkhateeb  wrote:

> Yup +1 for option 3, fix as you edit
>
> On Sep 13, 2016 1:16 PM, "Jacques Le Roux" 
> wrote:
>
> > Le 13/09/2016 à 11:56, Michael Brohl a écrit :
> >
> >> Same here. I'm not even sure if we really have clean groovy in the
> >> project, I assume it is mixed up with Java code in some areas.
> >>
> >> But I agree to have a consistent style and we should use the Groovy
> >> language as it shoul be used (even if I would have get used to it and
> like
> >> a a defined code line ending better).
> >>
> >> I see the following directions:
> >>
> >> 1. actively migrate to pure groovy and remove the semicolons (where
> >> applicable, it seems there are some cases where you need them, see
> >> https://dzone.com/articles/groovy-sometimes-you-still)
> >>
> >> 2. activeley put semicolons everywhere for consistency
> >>
> >> 3. do 1., but only when a groovy file is edited anyway. This would
> slowly
> >> migrate groovy files.
> >>
> >> I'd be in favor for 3., as long as there are other more important things
> >> to do or there is a volunteer to do it.
> >>
> >
> > This is what I somehow suggested, thanks for clarifying Michael! Better
> to
> > have consistent lines (with respect to semicolons) by file indeed.
> >
> > Jacques
> >
> >
> >> Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:
> >>
> >>> Okay I missed the historical context.
> >>>
> >>> Like Jacopo I also do not have a strong opinion, if it is easier and
> >>> faster
> >>> to keep them, then keep them. The important thing is to take a
> direction
> >>> and stay with it.
> >>>
> >>>
> >>>
> >>
> >>
> >
>


Re: svn commit: r1760528 - /ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control /ContextFilter.java

2016-09-13 Thread Jacques Le Roux

Works also locally here, I have forced a new build on BuildBot, could be a 
temporary error, happens rarely but happens.

Jacques


Le 13/09/2016 à 13:43, Jacopo Cappellato a écrit :

Yeah, thanks for the notification, I also saw the automatic build failure
email.
Weird, local tests are successful and that service doesn't seem to be
related to my last commit... but I am looking into it.

Jacopo

On Tue, Sep 13, 2016 at 1:15 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


You have got a small issue

https://ci.apache.org/builders/ofbiz-trunk/builds/1415

https://ci.apache.org/projects/ofbiz/logs/trunk/html/

Jacques



Le 13/09/2016 à 12:55, jaco...@apache.org a écrit :


Author: jacopoc
Date: Tue Sep 13 10:55:12 2016
New Revision: 1760528

URL: http://svn.apache.org/viewvc?rev=1760528&view=rev
Log:
Improved: set the character encoding to the system default (UTF-8) if and
only
if it is not set already by the client.

Before this change the filter used to override the character encoding,
even if
it was specified in the http request.
Also removed some unused imports and comments.


Modified:
  ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz
/webapp/control/ContextFilter.java

Modified: ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/sr
c/main/java/org/apache/ofbiz/webapp/control/ContextFilter.
java?rev=1760528&r1=1760527&r2=1760528&view=diff

==
--- ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java (original)
+++ ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java Tue Sep 13 10:55:12 2016
@@ -21,16 +21,13 @@ package org.apache.ofbiz.webapp.control;
   import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
   import java.util.Enumeration;
   import java.util.List;
   import java.util.Map;
-import java.util.Set;
 import javax.servlet.Filter;
   import javax.servlet.FilterChain;
   import javax.servlet.FilterConfig;
-import javax.servlet.ServletContext;
   import javax.servlet.ServletException;
   import javax.servlet.ServletRequest;
   import javax.servlet.ServletResponse;
@@ -50,12 +47,8 @@ import org.apache.ofbiz.entity.GenericVa
   import org.apache.ofbiz.entity.util.EntityQuery;
   import org.apache.ofbiz.entity.util.EntityUtil;
   import org.apache.ofbiz.security.Security;
-import org.apache.ofbiz.security.SecurityConfigurationException;
-import org.apache.ofbiz.security.SecurityFactory;
   import org.apache.ofbiz.service.LocalDispatcher;
-import org.apache.ofbiz.service.ServiceContainer;
   import org.apache.ofbiz.webapp.WebAppUtil;
-import org.apache.ofbiz.webapp.event.RequestBodyMapHandlerFactory;
   import org.apache.ofbiz.webapp.website.WebSiteWorker;
 /**
@@ -69,6 +62,9 @@ public class ContextFilter implements Fi
   protected FilterConfig config = null;
   protected boolean debug = false;
   +// default charset used to decode requests body data if no
encoding is specified in the request
+private String defaultCharacterEncoding;
+
   /**
* @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
*/
@@ -84,6 +80,10 @@ public class ContextFilter implements Fi
   debug = Debug.verboseOn();
   }
   +defaultCharacterEncoding = config.getServletContext().get
InitParameter("charset");
+if (UtilValidate.isEmpty(defaultCharacterEncoding)) {
+defaultCharacterEncoding = "UTF-8";
+}
   // check the serverId
   getServerId();
   // initialize the delegator
@@ -104,7 +104,6 @@ public class ContextFilter implements Fi
   HttpServletRequest httpRequest = (HttpServletRequest) request;
   HttpServletResponse httpResponse = (HttpServletResponse)
response;
   -// Debug.logInfo("Running ContextFilter.doFilter", module);
 // - Servlet Object Setup -
   @@ -250,10 +249,13 @@ public class ContextFilter implements Fi
   }
   }
   +if (request.getCharacterEncoding() == null) {
+request.setCharacterEncoding(defaultCharacterEncoding);
+}
+WebAppUtil.setAttributesFromRequestBody(request);
+
   // check if multi tenant is enabled
   boolean useMultitenant = EntityUtil.isMultiTenantEnabled();
-WebAppUtil.setCharacterEncoding(request);
-WebAppUtil.setAttributesFromRequestBody(request);
   if (useMultitenant) {
   // get tenant delegator by domain name
   String serverName = httpRequest.getServerName();








Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacopo Cappellato
I agree with Rishi's remarks: also, if we follow this approach then
functional changes will be buried in a bunch of non-functional changes.
This could work if the two are committed into two separate commits.

Jacopo

On Tue, Sep 13, 2016 at 2:45 PM, Rishi Solanki 
wrote:

> Fix as you edit, this is something like we are working on X functionality
> and to achieve that functionality if we want to edit an groovy file, then
> we will also remove/add semicolon to it.
>
> If I'm understanding it correctly, then -1 for it. As we have to ask
> explicitly to every contributor/committer to follow this practice on each
> commit/ticket.
>
> I'm up for #1 or #2 to actively remove/add semicolon. That is do it in one
> shot, not immediately but whenever we are ready to do it, otherwise with
> time we will have more inconsistency in groovy files on this parameter as
> semicolon.
>
> I'm not saying we must do it in one shot, but if community decides to
> proceed with any approach to actively add/remove semicolon then we (@HW)
> can try to assign single dev as volunteer to provide patch for all the
> files.
>
> Thanks!
>
> Best Regards,
> --
>
> Rishi Solanki
> Manager, Enterprise Software Development
> HotWax Systems Pvt. Ltd.
> Direct: +91-9893287847
> http://www.hotwaxsystems.com
>
> On Tue, Sep 13, 2016 at 3:49 PM, Taher Alkhateeb <
> slidingfilame...@gmail.com
> > wrote:
>
> > Yup +1 for option 3, fix as you edit
> >
> > On Sep 13, 2016 1:16 PM, "Jacques Le Roux"  >
> > wrote:
> >
> > > Le 13/09/2016 à 11:56, Michael Brohl a écrit :
> > >
> > >> Same here. I'm not even sure if we really have clean groovy in the
> > >> project, I assume it is mixed up with Java code in some areas.
> > >>
> > >> But I agree to have a consistent style and we should use the Groovy
> > >> language as it shoul be used (even if I would have get used to it and
> > like
> > >> a a defined code line ending better).
> > >>
> > >> I see the following directions:
> > >>
> > >> 1. actively migrate to pure groovy and remove the semicolons (where
> > >> applicable, it seems there are some cases where you need them, see
> > >> https://dzone.com/articles/groovy-sometimes-you-still)
> > >>
> > >> 2. activeley put semicolons everywhere for consistency
> > >>
> > >> 3. do 1., but only when a groovy file is edited anyway. This would
> > slowly
> > >> migrate groovy files.
> > >>
> > >> I'd be in favor for 3., as long as there are other more important
> things
> > >> to do or there is a volunteer to do it.
> > >>
> > >
> > > This is what I somehow suggested, thanks for clarifying Michael! Better
> > to
> > > have consistent lines (with respect to semicolons) by file indeed.
> > >
> > > Jacques
> > >
> > >
> > >> Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:
> > >>
> > >>> Okay I missed the historical context.
> > >>>
> > >>> Like Jacopo I also do not have a strong opinion, if it is easier and
> > >>> faster
> > >>> to keep them, then keep them. The important thing is to take a
> > direction
> > >>> and stay with it.
> > >>>
> > >>>
> > >>>
> > >>
> > >>
> > >
> >
>


Re: svn commit: r1760528 - /ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control /ContextFilter.java

2016-09-13 Thread Jacques Le Roux

Tests passed, must have been a BuildBot quirk

https://ci.apache.org/builders/ofbiz-trunk/builds/1416

Jacques


Le 13/09/2016 à 14:48, Jacques Le Roux a écrit :

Works also locally here, I have forced a new build on BuildBot, could be a 
temporary error, happens rarely but happens.

Jacques


Le 13/09/2016 à 13:43, Jacopo Cappellato a écrit :

Yeah, thanks for the notification, I also saw the automatic build failure
email.
Weird, local tests are successful and that service doesn't seem to be
related to my last commit... but I am looking into it.

Jacopo

On Tue, Sep 13, 2016 at 1:15 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


You have got a small issue

https://ci.apache.org/builders/ofbiz-trunk/builds/1415

https://ci.apache.org/projects/ofbiz/logs/trunk/html/

Jacques



Le 13/09/2016 à 12:55, jaco...@apache.org a écrit :


Author: jacopoc
Date: Tue Sep 13 10:55:12 2016
New Revision: 1760528

URL: http://svn.apache.org/viewvc?rev=1760528&view=rev
Log:
Improved: set the character encoding to the system default (UTF-8) if and
only
if it is not set already by the client.

Before this change the filter used to override the character encoding,
even if
it was specified in the http request.
Also removed some unused imports and comments.


Modified:
ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz
/webapp/control/ContextFilter.java

Modified: ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/sr
c/main/java/org/apache/ofbiz/webapp/control/ContextFilter.
java?rev=1760528&r1=1760527&r2=1760528&view=diff

==
--- ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java (original)
+++ ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
webapp/control/ContextFilter.java Tue Sep 13 10:55:12 2016
@@ -21,16 +21,13 @@ package org.apache.ofbiz.webapp.control;
   import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
   import java.util.Enumeration;
   import java.util.List;
   import java.util.Map;
-import java.util.Set;
 import javax.servlet.Filter;
   import javax.servlet.FilterChain;
   import javax.servlet.FilterConfig;
-import javax.servlet.ServletContext;
   import javax.servlet.ServletException;
   import javax.servlet.ServletRequest;
   import javax.servlet.ServletResponse;
@@ -50,12 +47,8 @@ import org.apache.ofbiz.entity.GenericVa
   import org.apache.ofbiz.entity.util.EntityQuery;
   import org.apache.ofbiz.entity.util.EntityUtil;
   import org.apache.ofbiz.security.Security;
-import org.apache.ofbiz.security.SecurityConfigurationException;
-import org.apache.ofbiz.security.SecurityFactory;
   import org.apache.ofbiz.service.LocalDispatcher;
-import org.apache.ofbiz.service.ServiceContainer;
   import org.apache.ofbiz.webapp.WebAppUtil;
-import org.apache.ofbiz.webapp.event.RequestBodyMapHandlerFactory;
   import org.apache.ofbiz.webapp.website.WebSiteWorker;
 /**
@@ -69,6 +62,9 @@ public class ContextFilter implements Fi
   protected FilterConfig config = null;
   protected boolean debug = false;
   +// default charset used to decode requests body data if no
encoding is specified in the request
+private String defaultCharacterEncoding;
+
   /**
* @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
*/
@@ -84,6 +80,10 @@ public class ContextFilter implements Fi
   debug = Debug.verboseOn();
   }
   +defaultCharacterEncoding = config.getServletContext().get
InitParameter("charset");
+if (UtilValidate.isEmpty(defaultCharacterEncoding)) {
+defaultCharacterEncoding = "UTF-8";
+}
   // check the serverId
   getServerId();
   // initialize the delegator
@@ -104,7 +104,6 @@ public class ContextFilter implements Fi
   HttpServletRequest httpRequest = (HttpServletRequest) request;
   HttpServletResponse httpResponse = (HttpServletResponse)
response;
   -// Debug.logInfo("Running ContextFilter.doFilter", module);
 // - Servlet Object Setup -
   @@ -250,10 +249,13 @@ public class ContextFilter implements Fi
   }
   }
   +if (request.getCharacterEncoding() == null) {
+ request.setCharacterEncoding(defaultCharacterEncoding);
+}
+WebAppUtil.setAttributesFromRequestBody(request);
+
   // check if multi tenant is enabled
   boolean useMultitenant = EntityUtil.isMultiTenantEnabled();
-WebAppUtil.setCharacterEncoding(request);
-WebAppUtil.setAttributesFromRequestBody(request);
   if (useMultitenant) {
   // get tenant delegator by domain name
   String serverName = httpRequest.getServerName();











Re: Groovy and semicolon at EOL

2016-09-13 Thread Michael Brohl

Good point, I hadn't thought of that!

So if we find a volunteer I would be for 1. (migrating to pure Groovy).

Michael

Am 13.09.16 um 14:52 schrieb Jacopo Cappellato:

I agree with Rishi's remarks: also, if we follow this approach then
functional changes will be buried in a bunch of non-functional changes.
This could work if the two are committed into two separate commits.

Jacopo







smime.p7s
Description: S/MIME Cryptographic Signature


Re: Groovy and semicolon at EOL

2016-09-13 Thread Taher Alkhateeb
Okay, given the priorities and work we have at the moment, I suggest we
keep semicolons and use it as the standard unless someone volunteers to
make a full switch. WDYT?

On Tue, Sep 13, 2016 at 3:52 PM, Jacopo Cappellato <
jacopo.cappell...@hotwaxsystems.com> wrote:

> I agree with Rishi's remarks: also, if we follow this approach then
> functional changes will be buried in a bunch of non-functional changes.
> This could work if the two are committed into two separate commits.
>
> Jacopo
>
> On Tue, Sep 13, 2016 at 2:45 PM, Rishi Solanki 
> wrote:
>
> > Fix as you edit, this is something like we are working on X functionality
> > and to achieve that functionality if we want to edit an groovy file, then
> > we will also remove/add semicolon to it.
> >
> > If I'm understanding it correctly, then -1 for it. As we have to ask
> > explicitly to every contributor/committer to follow this practice on each
> > commit/ticket.
> >
> > I'm up for #1 or #2 to actively remove/add semicolon. That is do it in
> one
> > shot, not immediately but whenever we are ready to do it, otherwise with
> > time we will have more inconsistency in groovy files on this parameter as
> > semicolon.
> >
> > I'm not saying we must do it in one shot, but if community decides to
> > proceed with any approach to actively add/remove semicolon then we (@HW)
> > can try to assign single dev as volunteer to provide patch for all the
> > files.
> >
> > Thanks!
> >
> > Best Regards,
> > --
> >
> > Rishi Solanki
> > Manager, Enterprise Software Development
> > HotWax Systems Pvt. Ltd.
> > Direct: +91-9893287847
> > http://www.hotwaxsystems.com
> >
> > On Tue, Sep 13, 2016 at 3:49 PM, Taher Alkhateeb <
> > slidingfilame...@gmail.com
> > > wrote:
> >
> > > Yup +1 for option 3, fix as you edit
> > >
> > > On Sep 13, 2016 1:16 PM, "Jacques Le Roux" <
> jacques.le.r...@les7arts.com
> > >
> > > wrote:
> > >
> > > > Le 13/09/2016 à 11:56, Michael Brohl a écrit :
> > > >
> > > >> Same here. I'm not even sure if we really have clean groovy in the
> > > >> project, I assume it is mixed up with Java code in some areas.
> > > >>
> > > >> But I agree to have a consistent style and we should use the Groovy
> > > >> language as it shoul be used (even if I would have get used to it
> and
> > > like
> > > >> a a defined code line ending better).
> > > >>
> > > >> I see the following directions:
> > > >>
> > > >> 1. actively migrate to pure groovy and remove the semicolons (where
> > > >> applicable, it seems there are some cases where you need them, see
> > > >> https://dzone.com/articles/groovy-sometimes-you-still)
> > > >>
> > > >> 2. activeley put semicolons everywhere for consistency
> > > >>
> > > >> 3. do 1., but only when a groovy file is edited anyway. This would
> > > slowly
> > > >> migrate groovy files.
> > > >>
> > > >> I'd be in favor for 3., as long as there are other more important
> > things
> > > >> to do or there is a volunteer to do it.
> > > >>
> > > >
> > > > This is what I somehow suggested, thanks for clarifying Michael!
> Better
> > > to
> > > > have consistent lines (with respect to semicolons) by file indeed.
> > > >
> > > > Jacques
> > > >
> > > >
> > > >> Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:
> > > >>
> > > >>> Okay I missed the historical context.
> > > >>>
> > > >>> Like Jacopo I also do not have a strong opinion, if it is easier
> and
> > > >>> faster
> > > >>> to keep them, then keep them. The important thing is to take a
> > > direction
> > > >>> and stay with it.
> > > >>>
> > > >>>
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> >
>


buildbot success in on ofbiz-trunk

2016-09-13 Thread buildbot
The Buildbot has detected a restored build on builder ofbiz-trunk while 
building . Full details are available at:
https://ci.apache.org/builders/ofbiz-trunk/builds/1416

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: orcus_ubuntu

Build Reason: forced: by IRC user  (privmsg): forces manual build 
after weird test failure
Build Source Stamp: HEAD
Blamelist: 

Build succeeded!

Sincerely,
 -The Buildbot





Re: svn commit: r1760528 - /ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control /ContextFilter.java

2016-09-13 Thread Jacopo Cappellato
cool, thanks

Jacopo

On Tue, Sep 13, 2016 at 2:56 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Tests passed, must have been a BuildBot quirk
>
> https://ci.apache.org/builders/ofbiz-trunk/builds/1416
>
> Jacques
>
>
>
> Le 13/09/2016 à 14:48, Jacques Le Roux a écrit :
>
>> Works also locally here, I have forced a new build on BuildBot, could be
>> a temporary error, happens rarely but happens.
>>
>> Jacques
>>
>>
>> Le 13/09/2016 à 13:43, Jacopo Cappellato a écrit :
>>
>>> Yeah, thanks for the notification, I also saw the automatic build failure
>>> email.
>>> Weird, local tests are successful and that service doesn't seem to be
>>> related to my last commit... but I am looking into it.
>>>
>>> Jacopo
>>>
>>> On Tue, Sep 13, 2016 at 1:15 PM, Jacques Le Roux <
>>> jacques.le.r...@les7arts.com> wrote:
>>>
>>> You have got a small issue

 https://ci.apache.org/builders/ofbiz-trunk/builds/1415

 https://ci.apache.org/projects/ofbiz/logs/trunk/html/

 Jacques



 Le 13/09/2016 à 12:55, jaco...@apache.org a écrit :

 Author: jacopoc
> Date: Tue Sep 13 10:55:12 2016
> New Revision: 1760528
>
> URL: http://svn.apache.org/viewvc?rev=1760528&view=rev
> Log:
> Improved: set the character encoding to the system default (UTF-8) if
> and
> only
> if it is not set already by the client.
>
> Before this change the filter used to override the character encoding,
> even if
> it was specified in the http request.
> Also removed some unused imports and comments.
>
>
> Modified:
> ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz
> /webapp/control/ContextFilter.java
>
> Modified: ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
> webapp/control/ContextFilter.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/sr
> c/main/java/org/apache/ofbiz/webapp/control/ContextFilter.
> java?rev=1760528&r1=1760527&r2=1760528&view=diff
> 
> ==
> --- ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
> webapp/control/ContextFilter.java (original)
> +++ ofbiz/trunk/framework/webapp/src/main/java/org/apache/ofbiz/
> webapp/control/ContextFilter.java Tue Sep 13 10:55:12 2016
> @@ -21,16 +21,13 @@ package org.apache.ofbiz.webapp.control;
>import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
>  import java.io.IOException;
> -import java.io.UnsupportedEncodingException;
>import java.util.Enumeration;
>import java.util.List;
>import java.util.Map;
> -import java.util.Set;
>  import javax.servlet.Filter;
>import javax.servlet.FilterChain;
>import javax.servlet.FilterConfig;
> -import javax.servlet.ServletContext;
>import javax.servlet.ServletException;
>import javax.servlet.ServletRequest;
>import javax.servlet.ServletResponse;
> @@ -50,12 +47,8 @@ import org.apache.ofbiz.entity.GenericVa
>import org.apache.ofbiz.entity.util.EntityQuery;
>import org.apache.ofbiz.entity.util.EntityUtil;
>import org.apache.ofbiz.security.Security;
> -import org.apache.ofbiz.security.SecurityConfigurationException;
> -import org.apache.ofbiz.security.SecurityFactory;
>import org.apache.ofbiz.service.LocalDispatcher;
> -import org.apache.ofbiz.service.ServiceContainer;
>import org.apache.ofbiz.webapp.WebAppUtil;
> -import org.apache.ofbiz.webapp.event.RequestBodyMapHandlerFactory;
>import org.apache.ofbiz.webapp.website.WebSiteWorker;
>  /**
> @@ -69,6 +62,9 @@ public class ContextFilter implements Fi
>protected FilterConfig config = null;
>protected boolean debug = false;
>+// default charset used to decode requests body data if no
> encoding is specified in the request
> +private String defaultCharacterEncoding;
> +
>/**
> * @see javax.servlet.Filter#init(javax.servlet.FilterConfig)
> */
> @@ -84,6 +80,10 @@ public class ContextFilter implements Fi
>debug = Debug.verboseOn();
>}
>+defaultCharacterEncoding = config.getServletContext().get
> InitParameter("charset");
> +if (UtilValidate.isEmpty(defaultCharacterEncoding)) {
> +defaultCharacterEncoding = "UTF-8";
> +}
>// check the serverId
>getServerId();
>// initialize the delegator
> @@ -104,7 +104,6 @@ public class ContextFilter implements Fi
>HttpServletRequest httpRequest = (HttpServletRequest)
> request;
>HttpServletResponse httpResponse = (HttpServletResponse)
> response;
>-// Debug.logInfo("Running ContextFilter.doFilter"

Re: Groovy and semicolon at EOL

2016-09-13 Thread Michael Brohl

+1

Michael

Am 13.09.16 um 14:58 schrieb Taher Alkhateeb:

Okay, given the priorities and work we have at the moment, I suggest we
keep semicolons and use it as the standard unless someone volunteers to
make a full switch. WDYT?

On Tue, Sep 13, 2016 at 3:52 PM, Jacopo Cappellato <
jacopo.cappell...@hotwaxsystems.com> wrote:


I agree with Rishi's remarks: also, if we follow this approach then
functional changes will be buried in a bunch of non-functional changes.
This could work if the two are committed into two separate commits.

Jacopo

On Tue, Sep 13, 2016 at 2:45 PM, Rishi Solanki 
wrote:







smime.p7s
Description: S/MIME Cryptographic Signature


Re: Groovy and semicolon at EOL

2016-09-13 Thread Rishi Solanki
+1 Taher, until we will have complete switch to pure groovy we should keep
the semicolon as practice.
+1 Michael, for migrating to pure Groovy.

We would try to assign dev for it and log Jira ticket accordingly.

Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com

On Tue, Sep 13, 2016 at 6:28 PM, Taher Alkhateeb  wrote:

> Okay, given the priorities and work we have at the moment, I suggest we
> keep semicolons and use it as the standard unless someone volunteers to
> make a full switch. WDYT?
>
> On Tue, Sep 13, 2016 at 3:52 PM, Jacopo Cappellato <
> jacopo.cappell...@hotwaxsystems.com> wrote:
>
> > I agree with Rishi's remarks: also, if we follow this approach then
> > functional changes will be buried in a bunch of non-functional changes.
> > This could work if the two are committed into two separate commits.
> >
> > Jacopo
> >
> > On Tue, Sep 13, 2016 at 2:45 PM, Rishi Solanki 
> > wrote:
> >
> > > Fix as you edit, this is something like we are working on X
> functionality
> > > and to achieve that functionality if we want to edit an groovy file,
> then
> > > we will also remove/add semicolon to it.
> > >
> > > If I'm understanding it correctly, then -1 for it. As we have to ask
> > > explicitly to every contributor/committer to follow this practice on
> each
> > > commit/ticket.
> > >
> > > I'm up for #1 or #2 to actively remove/add semicolon. That is do it in
> > one
> > > shot, not immediately but whenever we are ready to do it, otherwise
> with
> > > time we will have more inconsistency in groovy files on this parameter
> as
> > > semicolon.
> > >
> > > I'm not saying we must do it in one shot, but if community decides to
> > > proceed with any approach to actively add/remove semicolon then we
> (@HW)
> > > can try to assign single dev as volunteer to provide patch for all the
> > > files.
> > >
> > > Thanks!
> > >
> > > Best Regards,
> > > --
> > >
> > > Rishi Solanki
> > > Manager, Enterprise Software Development
> > > HotWax Systems Pvt. Ltd.
> > > Direct: +91-9893287847
> > > http://www.hotwaxsystems.com
> > >
> > > On Tue, Sep 13, 2016 at 3:49 PM, Taher Alkhateeb <
> > > slidingfilame...@gmail.com
> > > > wrote:
> > >
> > > > Yup +1 for option 3, fix as you edit
> > > >
> > > > On Sep 13, 2016 1:16 PM, "Jacques Le Roux" <
> > jacques.le.r...@les7arts.com
> > > >
> > > > wrote:
> > > >
> > > > > Le 13/09/2016 à 11:56, Michael Brohl a écrit :
> > > > >
> > > > >> Same here. I'm not even sure if we really have clean groovy in the
> > > > >> project, I assume it is mixed up with Java code in some areas.
> > > > >>
> > > > >> But I agree to have a consistent style and we should use the
> Groovy
> > > > >> language as it shoul be used (even if I would have get used to it
> > and
> > > > like
> > > > >> a a defined code line ending better).
> > > > >>
> > > > >> I see the following directions:
> > > > >>
> > > > >> 1. actively migrate to pure groovy and remove the semicolons
> (where
> > > > >> applicable, it seems there are some cases where you need them, see
> > > > >> https://dzone.com/articles/groovy-sometimes-you-still)
> > > > >>
> > > > >> 2. activeley put semicolons everywhere for consistency
> > > > >>
> > > > >> 3. do 1., but only when a groovy file is edited anyway. This would
> > > > slowly
> > > > >> migrate groovy files.
> > > > >>
> > > > >> I'd be in favor for 3., as long as there are other more important
> > > things
> > > > >> to do or there is a volunteer to do it.
> > > > >>
> > > > >
> > > > > This is what I somehow suggested, thanks for clarifying Michael!
> > Better
> > > > to
> > > > > have consistent lines (with respect to semicolons) by file indeed.
> > > > >
> > > > > Jacques
> > > > >
> > > > >
> > > > >> Am 13.09.16 um 08:49 schrieb Taher Alkhateeb:
> > > > >>
> > > > >>> Okay I missed the historical context.
> > > > >>>
> > > > >>> Like Jacopo I also do not have a strong opinion, if it is easier
> > and
> > > > >>> faster
> > > > >>> to keep them, then keep them. The important thing is to take a
> > > > direction
> > > > >>> and stay with it.
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > >
> > >
> >
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Michael Brohl

Thanks, Rishi!

Am 13.09.16 um 15:10 schrieb Rishi Solanki:

+1 Taher, until we will have complete switch to pure groovy we should keep
the semicolon as practice.
+1 Michael, for migrating to pure Groovy.

We would try to assign dev for it and log Jira ticket accordingly.

Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com







smime.p7s
Description: S/MIME Cryptographic Signature


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacopo Cappellato
On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> ...
> Before applying a such change, I'd really like to know if everybody is
> aware of what that means when it comes to svn annotations. I repeat: we
> will then lose all the svn annotations history in all the Groovy files. ...
>

Jacques, are you aware that you can pass the -r argument to the
blame/annotate command?

Jacopo


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Sorry, I started this thread by asking this question:

>While committing r1760406  I wondered if I should really put semicolons at end 
of Groovy files lines.
>We know it's useless in Groovy. Should we continue to put them, and if yes for 
which reasons?

The question switched to "should we remove all the trailing semicolons from all the 
Groovy files"

We all know it's an easy task using a S/R regexp to remove all the trailing semicolons from all the Groovy files in one shoot. Or maybe for easier 
reviews in several shoots (by component? But who will really review these changes?)


So I don't feel we have answered my question but we rather sidetracked to a solution I did not ask about. This because of the possible end of lines 
inconsistency in Groovy files.


Before applying a such change, I'd really like to know if everybody is aware of what that means when it comes to svn annotations. I repeat: we will 
then lose all the svn annotations history in all the Groovy files.

And that seems to me to be more a concern than consistency in Groovy files!

To get the idea: unrelated but close, we decided to move files around in OFBiz, OK. But now backporting bug fixes is a pain. You have to change files 
paths by hand. This is the kind of changes that must thought thorough before taking a decision.


Thanks

Jacques


Le 13/09/2016 à 16:17, Michael Brohl a écrit :

Thanks, Rishi!

Am 13.09.16 um 15:10 schrieb Rishi Solanki:

+1 Taher, until we will have complete switch to pure groovy we should keep
the semicolon as practice.
+1 Michael, for migrating to pure Groovy.

We would try to assign dev for it and log Jira ticket accordingly.

Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com









Re: Groovy and semicolon at EOL

2016-09-13 Thread Rishi Solanki
Agreed on the fact that its an pain to backport the bug fixes to releases.
Especially when we have to change something manually and it has been done
with many files in last few months i.e bulk changes with all files of xType.

I'm not sure, but what is the good time to do such changes (may be just
before the next release?). Or we should port such changes which are for
consistency to releases. Or may be its fine to keep it as is.

Thanks!



Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com

On Tue, Sep 13, 2016 at 8:15 PM, Jacopo Cappellato <
jacopo.cappell...@hotwaxsystems.com> wrote:

> On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
> jacques.le.r...@les7arts.com> wrote:
>
> > ...
> > Before applying a such change, I'd really like to know if everybody is
> > aware of what that means when it comes to svn annotations. I repeat: we
> > will then lose all the svn annotations history in all the Groovy files.
> ...
> >
>
> Jacques, are you aware that you can pass the -r argument to the
> blame/annotate command?
>
> Jacopo
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :

On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


...
Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files. ...


Jacques, are you aware that you can pass the -r argument to the
blame/annotate command?

Jacopo

I must say I never use that when looking at annotations in a file in Eclipse. It's maybe useful in certain circumstances, but I hardly see when. And 
once all the lines a file has been modified in one commit, I guess -r does not help at all, anyway you get only this information. Or do I miss 
something? Should I know the revision I'm looking for? I rather try to know when and why a line has been changed, what are the reasons of these 
changes, maybe to find an related Jira, etc.


Jacques



Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacopo Cappellato
Some examples:

svn blame README.md

after review you run

svn blame README.md -r 1:1757044

and then

svn blame README.md -r 1:1757042

and so on to get back in history... nothing is lost, annotations are always
there.

Jacopo

PS: I think there is some trick to do the same with TortoiseSVN but I can't
tell you the details since I don't use it

On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :
>
>> On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
>> jacques.le.r...@les7arts.com> wrote:
>>
>> ...
>>> Before applying a such change, I'd really like to know if everybody is
>>> aware of what that means when it comes to svn annotations. I repeat: we
>>> will then lose all the svn annotations history in all the Groovy files.
>>> ...
>>>
>>> Jacques, are you aware that you can pass the -r argument to the
>> blame/annotate command?
>>
>> Jacopo
>>
>> I must say I never use that when looking at annotations in a file in
> Eclipse. It's maybe useful in certain circumstances, but I hardly see when.
> And once all the lines a file has been modified in one commit, I guess -r
> does not help at all, anyway you get only this information. Or do I miss
> something? Should I know the revision I'm looking for? I rather try to know
> when and why a line has been changed, what are the reasons of these
> changes, maybe to find an related Jira, etc.
>
> Jacques
>
>


Re: Some questions on plugin system

2016-09-13 Thread Taher Alkhateeb
Hello Folks,

After quite a bit of work, I have a first working PoC for the plugin system
with the following highlights:

- Plugins are OFBiz components that reside in /specialpurpose (hopefully
renamed to /plugins later)
- Plugins can be published to maven repositories and retrieved from maven
repositories
- Plugins can have dependencies on other plugins
- I created a minimal set of tasks that do the essentials: createPlugin,
installPlugin, uninstallPlugin, pullPlugin (install from maven repo) and
publishPlugin (publish it to maven repo on localhost)
- I provided documentation in README.md

I appreciate your help in feedback, ideas, testing and sharing whatever is
on your mind.

You will find the patch in https://issues.apache.org/jira/browse/OFBIZ-7972

Cheers,

Taher Alkhateeb



On Fri, Sep 9, 2016 at 12:55 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Le 09/09/2016 à 10:32, Jacopo Cappellato a écrit :
>
>> On Thu, Sep 8, 2016 at 11:31 PM, Jacques Le Roux <
>> jacques.le.r...@les7arts.com> wrote:
>>
>> ...
>>> So it would be easier for us (OFBiz team) and contributors to deliver (at
>>> least free) plugins [...]
>>>
>>
>> The terms "us", "OFBiz team" and the distinction with "contributors" don't
>> make much sense to me and can cause confusion: there is just one "OFBiz
>> community" in which everyone can contribute with ideas, work, code... and
>> plugins.
>>
>> Jacopo
>>
>> Yes you are right, and actually, as Taher outlined, the
> components/plugins provided by OFBiz OOTB would not fit in the possible use
> of JitPack anyway.
>
> Jacques
>
>


Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Thanks Jacopo,

I found how to use it in TortoiseSVN (it starts from the log view)
It's complementary to what Subclipse gives and so interesting but not 
comparable.

You don't have this global view Subclipse offers with each annotation by line 
from start (r1) to HEAD.
Very useful with colored annotations in the same column than lines numbers. But it unfortunately contains only the last revision if all lines have 
been modified together in that revision.

Note: to see it you need to use "Show Quick Diff" ("Revision" and "Combined 
Colors" are then default options, hovering is enough for me).
Same than you decide to show line numbers in this column... More for those who 
are still using Eclipse...

Jacques

Le 13/09/2016 à 17:40, Jacopo Cappellato a écrit :

Some examples:

svn blame README.md

after review you run

svn blame README.md -r 1:1757044

and then

svn blame README.md -r 1:1757042

and so on to get back in history... nothing is lost, annotations are always
there.

Jacopo

PS: I think there is some trick to do the same with TortoiseSVN but I can't
tell you the details since I don't use it

On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :


On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

...

Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files.
...

Jacques, are you aware that you can pass the -r argument to the

blame/annotate command?

Jacopo

I must say I never use that when looking at annotations in a file in

Eclipse. It's maybe useful in certain circumstances, but I hardly see when.
And once all the lines a file has been modified in one commit, I guess -r
does not help at all, anyway you get only this information. Or do I miss
something? Should I know the revision I'm looking for? I rather try to know
when and why a line has been changed, what are the reasons of these
changes, maybe to find an related Jira, etc.

Jacques






Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

BTW thinking about it, don't you have something similar in IntellIJ?

I found an (old) image there 
https://markphip.blogspot.fr/2006/12/subclipse-live-annotations.html

Jacques


Le 13/09/2016 à 20:16, Jacques Le Roux a écrit :

Thanks Jacopo,

I found how to use it in TortoiseSVN (it starts from the log view)
It's complementary to what Subclipse gives and so interesting but not 
comparable.

You don't have this global view Subclipse offers with each annotation by line 
from start (r1) to HEAD.
Very useful with colored annotations in the same column than lines numbers. But it unfortunately contains only the last revision if all lines have 
been modified together in that revision.

Note: to see it you need to use "Show Quick Diff" ("Revision" and "Combined 
Colors" are then default options, hovering is enough for me).
Same than you decide to show line numbers in this column... More for those who 
are still using Eclipse...

Jacques

Le 13/09/2016 à 17:40, Jacopo Cappellato a écrit :

Some examples:

svn blame README.md

after review you run

svn blame README.md -r 1:1757044

and then

svn blame README.md -r 1:1757042

and so on to get back in history... nothing is lost, annotations are always
there.

Jacopo

PS: I think there is some trick to do the same with TortoiseSVN but I can't
tell you the details since I don't use it

On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :


On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

...

Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files.
...

Jacques, are you aware that you can pass the -r argument to the

blame/annotate command?

Jacopo

I must say I never use that when looking at annotations in a file in

Eclipse. It's maybe useful in certain circumstances, but I hardly see when.
And once all the lines a file has been modified in one commit, I guess -r
does not help at all, anyway you get only this information. Or do I miss
something? Should I know the revision I'm looking for? I rather try to know
when and why a line has been changed, what are the reasons of these
changes, maybe to find an related Jira, etc.

Jacques









Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 17:06, Rishi Solanki a écrit :

Agreed on the fact that its an pain to backport the bug fixes to releases.
Especially when we have to change something manually and it has been done
with many files in last few months i.e bulk changes with all files of xType.

I'm not sure, but what is the good time to do such changes (may be just
before the next release?). Or we should port such changes which are for
consistency to releases. Or may be its fine to keep it as is.


I'd finally prefer to keep it as is. At least that's my opinion for now.

Jacques



Thanks!



Rishi Solanki
Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com

On Tue, Sep 13, 2016 at 8:15 PM, Jacopo Cappellato <
jacopo.cappell...@hotwaxsystems.com> wrote:


On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


...
Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files.

...
Jacques, are you aware that you can pass the -r argument to the
blame/annotate command?

Jacopo





Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

OK found that the same than in Subclipse also exists in TortoiseSVN

But you need to use a command line (weird for a GUI), eg (from TortoiseSVN root 
folder)

TortoiseProc.exe /command:blame 
/path:"C:\projectASF-Mars\ofbiz\applications\product\src\main\java\org\apache\ofbiz\product\catalog\CatalogWorker.java"

All is explained here 
https://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-automation.html#tsvn-automation-basics

From the resulting UI (comparable to Subclipse) I guess changing all lines of a 
file will have the same effect.
Even if indeed the annotations are not lost, they are very hard to use if you 
need to compare revision by revision.

Jacques


Le 13/09/2016 à 20:21, Jacques Le Roux a écrit :

BTW thinking about it, don't you have something similar in IntellIJ?

I found an (old) image there 
https://markphip.blogspot.fr/2006/12/subclipse-live-annotations.html

Jacques


Le 13/09/2016 à 20:16, Jacques Le Roux a écrit :

Thanks Jacopo,

I found how to use it in TortoiseSVN (it starts from the log view)
It's complementary to what Subclipse gives and so interesting but not 
comparable.

You don't have this global view Subclipse offers with each annotation by line 
from start (r1) to HEAD.
Very useful with colored annotations in the same column than lines numbers. But it unfortunately contains only the last revision if all lines have 
been modified together in that revision.

Note: to see it you need to use "Show Quick Diff" ("Revision" and "Combined 
Colors" are then default options, hovering is enough for me).
Same than you decide to show line numbers in this column... More for those who 
are still using Eclipse...

Jacques

Le 13/09/2016 à 17:40, Jacopo Cappellato a écrit :

Some examples:

svn blame README.md

after review you run

svn blame README.md -r 1:1757044

and then

svn blame README.md -r 1:1757042

and so on to get back in history... nothing is lost, annotations are always
there.

Jacopo

PS: I think there is some trick to do the same with TortoiseSVN but I can't
tell you the details since I don't use it

On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :


On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

...

Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files.
...

Jacques, are you aware that you can pass the -r argument to the

blame/annotate command?

Jacopo

I must say I never use that when looking at annotations in a file in

Eclipse. It's maybe useful in certain circumstances, but I hardly see when.
And once all the lines a file has been modified in one commit, I guess -r
does not help at all, anyway you get only this information. Or do I miss
something? Should I know the revision I'm looking for? I rather try to know
when and why a line has been changed, what are the reasons of these
changes, maybe to find an related Jira, etc.

Jacques












Re: Groovy and semicolon at EOL

2016-09-13 Thread Jacques Le Roux

Le 13/09/2016 à 21:28, Jacques Le Roux a écrit :

OK found that the same than in Subclipse also exists in TortoiseSVN

But you need to use a command line (weird for a GUI), eg (from TortoiseSVN root 
folder)


Actually wrong, simply pick a file in Windows file explorer using TortoiseSVN  
context menu, et voilà!
I confirm, totally comparable to Subclipse annotations

Jacques



TortoiseProc.exe /command:blame 
/path:"C:\projectASF-Mars\ofbiz\applications\product\src\main\java\org\apache\ofbiz\product\catalog\CatalogWorker.java"

All is explained here 
https://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-automation.html#tsvn-automation-basics

From the resulting UI (comparable to Subclipse) I guess changing all lines of a 
file will have the same effect.
Even if indeed the annotations are not lost, they are very hard to use if you 
need to compare revision by revision.

Jacques


Le 13/09/2016 à 20:21, Jacques Le Roux a écrit :

BTW thinking about it, don't you have something similar in IntellIJ?

I found an (old) image there 
https://markphip.blogspot.fr/2006/12/subclipse-live-annotations.html

Jacques


Le 13/09/2016 à 20:16, Jacques Le Roux a écrit :

Thanks Jacopo,

I found how to use it in TortoiseSVN (it starts from the log view)
It's complementary to what Subclipse gives and so interesting but not 
comparable.

You don't have this global view Subclipse offers with each annotation by line 
from start (r1) to HEAD.
Very useful with colored annotations in the same column than lines numbers. But it unfortunately contains only the last revision if all lines have 
been modified together in that revision.

Note: to see it you need to use "Show Quick Diff" ("Revision" and "Combined 
Colors" are then default options, hovering is enough for me).
Same than you decide to show line numbers in this column... More for those who 
are still using Eclipse...

Jacques

Le 13/09/2016 à 17:40, Jacopo Cappellato a écrit :

Some examples:

svn blame README.md

after review you run

svn blame README.md -r 1:1757044

and then

svn blame README.md -r 1:1757042

and so on to get back in history... nothing is lost, annotations are always
there.

Jacopo

PS: I think there is some trick to do the same with TortoiseSVN but I can't
tell you the details since I don't use it

On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :


On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

...

Before applying a such change, I'd really like to know if everybody is
aware of what that means when it comes to svn annotations. I repeat: we
will then lose all the svn annotations history in all the Groovy files.
...

Jacques, are you aware that you can pass the -r argument to the

blame/annotate command?

Jacopo

I must say I never use that when looking at annotations in a file in

Eclipse. It's maybe useful in certain circumstances, but I hardly see when.
And once all the lines a file has been modified in one commit, I guess -r
does not help at all, anyway you get only this information. Or do I miss
something? Should I know the revision I'm looking for? I rather try to know
when and why a line has been changed, what are the reasons of these
changes, maybe to find an related Jira, etc.

Jacques















Re: Groovy and semicolon at EOL

2016-09-13 Thread Scott Gray
I don't particularly care one way or another if groovy files have a
semi-colon at the end.  I don't even care about consistency because it is
such a minor thing.

I say remove them if they're on a line you happen to be editing, otherwise
just leave them be.

Regarding the annotations, there's plenty of ways to search commit logs and
personally I've never found blame to be very useful.  I don't think it
should be a reason to block any future bulk S/R cleanups.  We've had plenty
in the past (Double -> BigDecimal, Delegator -> EntityQuery, whitespace
removal, etc.) and we should continue to do it to keep things clean.

For searching diffs, before using git-svn I used to use: svn log -diff
 and then use the search in the terminal to find the string
I'm looking for.

Regards
Scott

On 14 September 2016 at 07:33, Jacques Le Roux  wrote:

> Le 13/09/2016 à 21:28, Jacques Le Roux a écrit :
>
>> OK found that the same than in Subclipse also exists in TortoiseSVN
>>
>> But you need to use a command line (weird for a GUI), eg (from
>> TortoiseSVN root folder)
>>
>
> Actually wrong, simply pick a file in Windows file explorer using
> TortoiseSVN  context menu, et voilà!
> I confirm, totally comparable to Subclipse annotations
>
> Jacques
>
>
>
>> TortoiseProc.exe /command:blame /path:"C:\projectASF-Mars\ofbi
>> z\applications\product\src\main\java\org\apache\ofbiz\
>> product\catalog\CatalogWorker.java"
>>
>> All is explained here https://tortoisesvn.net/docs/r
>> elease/TortoiseSVN_en/tsvn-automation.html#tsvn-automation-basics
>>
>> From the resulting UI (comparable to Subclipse) I guess changing all
>> lines of a file will have the same effect.
>> Even if indeed the annotations are not lost, they are very hard to use if
>> you need to compare revision by revision.
>>
>> Jacques
>>
>>
>> Le 13/09/2016 à 20:21, Jacques Le Roux a écrit :
>>
>>> BTW thinking about it, don't you have something similar in IntellIJ?
>>>
>>> I found an (old) image there https://markphip.blogspot.fr/2
>>> 006/12/subclipse-live-annotations.html
>>>
>>> Jacques
>>>
>>>
>>> Le 13/09/2016 à 20:16, Jacques Le Roux a écrit :
>>>
 Thanks Jacopo,

 I found how to use it in TortoiseSVN (it starts from the log view)
 It's complementary to what Subclipse gives and so interesting but not
 comparable.

 You don't have this global view Subclipse offers with each annotation
 by line from start (r1) to HEAD.
 Very useful with colored annotations in the same column than lines
 numbers. But it unfortunately contains only the last revision if all lines
 have been modified together in that revision.
 Note: to see it you need to use "Show Quick Diff" ("Revision" and
 "Combined Colors" are then default options, hovering is enough for me).
 Same than you decide to show line numbers in this column... More for
 those who are still using Eclipse...

 Jacques

 Le 13/09/2016 à 17:40, Jacopo Cappellato a écrit :

> Some examples:
>
> svn blame README.md
>
> after review you run
>
> svn blame README.md -r 1:1757044
>
> and then
>
> svn blame README.md -r 1:1757042
>
> and so on to get back in history... nothing is lost, annotations are
> always
> there.
>
> Jacopo
>
> PS: I think there is some trick to do the same with TortoiseSVN but I
> can't
> tell you the details since I don't use it
>
> On Tue, Sep 13, 2016 at 5:16 PM, Jacques Le Roux <
> jacques.le.r...@les7arts.com> wrote:
>
> Le 13/09/2016 à 16:45, Jacopo Cappellato a écrit :
>>
>> On Tue, Sep 13, 2016 at 4:36 PM, Jacques Le Roux <
>>> jacques.le.r...@les7arts.com> wrote:
>>>
>>> ...
>>>
 Before applying a such change, I'd really like to know if everybody
 is
 aware of what that means when it comes to svn annotations. I
 repeat: we
 will then lose all the svn annotations history in all the Groovy
 files.
 ...

 Jacques, are you aware that you can pass the -r argument to the

>>> blame/annotate command?
>>>
>>> Jacopo
>>>
>>> I must say I never use that when looking at annotations in a file in
>>>
>> Eclipse. It's maybe useful in certain circumstances, but I hardly see
>> when.
>> And once all the lines a file has been modified in one commit, I
>> guess -r
>> does not help at all, anyway you get only this information. Or do I
>> miss
>> something? Should I know the revision I'm looking for? I rather try
>> to know
>> when and why a line has been changed, what are the reasons of these
>> changes, maybe to find an related Jira, etc.
>>
>> Jacques
>>
>>
>>


>>>
>>>
>>
>>
>