You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@olingo.apache.org by "V.A, Chandan" <ch...@sap.com> on 2013/08/05 11:49:45 UTC

RE: code style

Hi Stephan,
Can we have line wrapping policy for code-style formatter set to "Wrap where necessary" or can we have a defined line width of 80 chars. This could avoid scrolling.

Thanks,
Kind Regards
Chandan VA

-----Original Message-----
From: Klevenz, Stephan [mailto:stephan.klevenz@sap.com] 
Sent: Wednesday, July 31, 2013 5:42 PM
To: dev@olingo.incubator.apache.org
Subject: code style

Hi,

The original code was implemented by using these code styles for Eclipse:

https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata2.git;a=tree;f=src/eclipse;h=420ec7b716ea24ccd66f4bfddf2c08ccf8c86f6c;hb=HEAD

I suggest to continue using them and do a clean up code before each commit. If someone does not agree to this settings then feel free to discuss and it is an option to change.

-- Stephan

BTW.: There are currently a lot of javadoc warnings in the build. Maybe everyone can have a look into this and get it fixed.

Re: code style

Posted by "Boehm, Tamara" <ta...@sap.com>.
Hi, 
I agree with a line width of 120 chars too.
Kind regards,
Tamara

On 8/7/13 12:18 PM, "Huesken, Jens" <je...@sap.com> wrote:

>Hi,
>I would move to "line wrapping policy" and "120 chars". I agree to
>Francesco that 120 seems to be quite commonly accepted.
>Best regards,
>Jens.
>
>-----Original Message-----
>From: Bolz, Michael [mailto:michael.bolz@sap.com]
>Sent: Mittwoch, 7. August 2013 06:15
>To: dev@olingo.incubator.apache.org
>Subject: Re: code style
>
>Hi,
>
>It will not be easy to find a solution which is acceptable for all.
>But currently it seems so:
>
>"line wrapping policy": +4
>"80 chars": +1 (if possible I would give here a -1/veto ;o)  )
>"135 chars": +1
>"135 chars": +2
>
>
>I think we should wait till this weekend and then decide by majority.
>Can everybody agree with this or is somebody opposed?
>
>Kind regard,
>Michael
>
>
>


RE: code style

Posted by "Huesken, Jens" <je...@sap.com>.
Hi,
I would move to "line wrapping policy" and "120 chars". I agree to Francesco that 120 seems to be quite commonly accepted.
Best regards,
Jens.

-----Original Message-----
From: Bolz, Michael [mailto:michael.bolz@sap.com] 
Sent: Mittwoch, 7. August 2013 06:15
To: dev@olingo.incubator.apache.org
Subject: Re: code style

Hi,

It will not be easy to find a solution which is acceptable for all.
But currently it seems so:

"line wrapping policy": +4
"80 chars": +1 (if possible I would give here a -1/veto ;o)  )
"135 chars": +1
"135 chars": +2


I think we should wait till this weekend and then decide by majority.
Can everybody agree with this or is somebody opposed?

Kind regard,
Michael



On 05.08.13 13:17, "Francesco Chicchiriccò" <il...@apache.org> wrote:

>Hi,
>I would personally move to 120 characters rather than 135: the former
>seems to be quite commonly accepted as extension of traditional 80.
>
>Regards.
>
>On 05/08/2013 12:54, Chauhan, Chitresh wrote:
>> Hi,
>> For me its :
>> Line wrapping policy(wrap when necessary) and 135 chars..
>>
>> Best regards,
>> Chitresh
>>
>> -----Original Message-----
>> From: Bolz, Michael [mailto:michael.bolz@sap.com]
>> Sent: Monday, August 05, 2013 3:58 PM
>> To: dev@olingo.incubator.apache.org
>> Subject: Re: code style
>>
>> Hi,
>>
>> In my opinion we could define a "line wrapping policy". But for me is a
>> line width of 80 chars much to low.
>> I think today normally at each monitor (> 19") you can handle ~135 chars
>> without scrolling.
>>
>> So for me:
>> +1: "line wrapping policy"
>> -1: "80 chars"
>> +1: "135 chars"
>>
>> Any other opinions?
>>
>> Kind regards,
>> Michael


Re: code style

Posted by "Bolz, Michael" <mi...@sap.com>.
Hi,

It will not be easy to find a solution which is acceptable for all.
But currently it seems so:

"line wrapping policy": +4
"80 chars": +1 (if possible I would give here a -1/veto ;o)  )
"135 chars": +1
"135 chars": +2


I think we should wait till this weekend and then decide by majority.
Can everybody agree with this or is somebody opposed?

Kind regard,
Michael



On 05.08.13 13:17, "Francesco Chicchiriccò" <il...@apache.org> wrote:

>Hi,
>I would personally move to 120 characters rather than 135: the former
>seems to be quite commonly accepted as extension of traditional 80.
>
>Regards.
>
>On 05/08/2013 12:54, Chauhan, Chitresh wrote:
>> Hi,
>> For me its :
>> Line wrapping policy(wrap when necessary) and 135 chars..
>>
>> Best regards,
>> Chitresh
>>
>> -----Original Message-----
>> From: Bolz, Michael [mailto:michael.bolz@sap.com]
>> Sent: Monday, August 05, 2013 3:58 PM
>> To: dev@olingo.incubator.apache.org
>> Subject: Re: code style
>>
>> Hi,
>>
>> In my opinion we could define a "line wrapping policy". But for me is a
>> line width of 80 chars much to low.
>> I think today normally at each monitor (> 19") you can handle ~135 chars
>> without scrolling.
>>
>> So for me:
>> +1: "line wrapping policy"
>> -1: "80 chars"
>> +1: "135 chars"
>>
>> Any other opinions?
>>
>> Kind regards,
>> Michael


Re: code style

Posted by Francesco Chicchiriccò <il...@apache.org>.
Hi,
I would personally move to 120 characters rather than 135: the former 
seems to be quite commonly accepted as extension of traditional 80.

Regards.

On 05/08/2013 12:54, Chauhan, Chitresh wrote:
> Hi,
> For me its :
> Line wrapping policy(wrap when necessary) and 135 chars..
>
> Best regards,
> Chitresh
>
> -----Original Message-----
> From: Bolz, Michael [mailto:michael.bolz@sap.com]
> Sent: Monday, August 05, 2013 3:58 PM
> To: dev@olingo.incubator.apache.org
> Subject: Re: code style
>
> Hi,
>
> In my opinion we could define a "line wrapping policy". But for me is a
> line width of 80 chars much to low.
> I think today normally at each monitor (> 19") you can handle ~135 chars
> without scrolling.
>
> So for me:
> +1: "line wrapping policy"
> -1: "80 chars"
> +1: "135 chars"
>
> Any other opinions?
>
> Kind regards,
> Michael
>
>
>
> On 05.08.13 11:49, "V.A, Chandan" <ch...@sap.com> wrote:
>
>> Hi Stephan,
>> Can we have line wrapping policy for code-style formatter set to "Wrap
>> where necessary" or can we have a defined line width of 80 chars. This
>> could avoid scrolling.
>>
>> Thanks,
>> Kind Regards
>> Chandan VA
>>
>> -----Original Message-----
>> From: Klevenz, Stephan [mailto:stephan.klevenz@sap.com]
>> Sent: Wednesday, July 31, 2013 5:42 PM
>> To: dev@olingo.incubator.apache.org
>> Subject: code style
>>
>> Hi,
>>
>> The original code was implemented by using these code styles for Eclipse:
>>
>> https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata2.git;a=tr
>> ee;f=src/eclipse;h=420ec7b716ea24ccd66f4bfddf2c08ccf8c86f6c;hb=HEAD
>>
>> I suggest to continue using them and do a clean up code before each
>> commit. If someone does not agree to this settings then feel free to
>> discuss and it is an option to change.
>>
>> -- Stephan
>>
>> BTW.: There are currently a lot of javadoc warnings in the build. Maybe
>> everyone can have a look into this and get it fixed.

-- 
Francesco Chicchiriccò

ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member
http://people.apache.org/~ilgrosso/


RE: code style

Posted by "Chauhan, Chitresh" <ch...@sap.com>.
Hi,
For me its :
Line wrapping policy(wrap when necessary) and 135 chars..

Best regards,
Chitresh

-----Original Message-----
From: Bolz, Michael [mailto:michael.bolz@sap.com] 
Sent: Monday, August 05, 2013 3:58 PM
To: dev@olingo.incubator.apache.org
Subject: Re: code style

Hi,

In my opinion we could define a "line wrapping policy". But for me is a
line width of 80 chars much to low.
I think today normally at each monitor (> 19") you can handle ~135 chars
without scrolling.

So for me:
+1: "line wrapping policy"
-1: "80 chars"
+1: "135 chars"

Any other opinions?

Kind regards,
Michael



On 05.08.13 11:49, "V.A, Chandan" <ch...@sap.com> wrote:

>Hi Stephan,
>Can we have line wrapping policy for code-style formatter set to "Wrap
>where necessary" or can we have a defined line width of 80 chars. This
>could avoid scrolling.
>
>Thanks,
>Kind Regards
>Chandan VA
>
>-----Original Message-----
>From: Klevenz, Stephan [mailto:stephan.klevenz@sap.com]
>Sent: Wednesday, July 31, 2013 5:42 PM
>To: dev@olingo.incubator.apache.org
>Subject: code style
>
>Hi,
>
>The original code was implemented by using these code styles for Eclipse:
>
>https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata2.git;a=tr
>ee;f=src/eclipse;h=420ec7b716ea24ccd66f4bfddf2c08ccf8c86f6c;hb=HEAD
>
>I suggest to continue using them and do a clean up code before each
>commit. If someone does not agree to this settings then feel free to
>discuss and it is an option to change.
>
>-- Stephan
>
>BTW.: There are currently a lot of javadoc warnings in the build. Maybe
>everyone can have a look into this and get it fixed.


Re: code style

Posted by Aki Yoshida <el...@gmail.com>.
Hi Michael,
thanks.

Maybe you might want to just take the stylecheck setting and also pmd
setting from other existing apache projects?

And I think it would be better to turn on these checks by default
install and also be used during code editing in eclipse, no? That way,
you won't postpone the fixing of the code or let others to fix. With
the fastinstall option provided, you can always turn them off when you
are in hurry and have a need for repeatedly mvn-building sub
components to test another component, etc.

regards, aki


2013/8/13 Bolz, Michael <mi...@sap.com>:
> Hi and Welcome Aki,
>
>
> The Tabs in the pom will be fixed with the "initial code clean up run".
> Which I think I will do till end of the week if nobody calls "veto".
>
> Our other points sounds interesting ("fast install") and should be
> discussed.
> The checkstyle plugin should only be executed within the "build.quality"
> profile.
> With other profiles it should be only executed if manually called (e.g.
> via "mvn checkstyle:check") because of the "<execution>" configuration.
> At least within my environment it works that way. Has anybody another
> experience?
>
>
> Nevertheless I think we can set the default goal to "install".
>
> Kind regards,
> Michael
>
> PS: Thanks for the test with cxf-2.7.6  ;o)
>
> On 13.08.13 15:05, "Aki Yoshida" <el...@gmail.com> wrote:
>
>>Hi,
>>first time posting here :-)
>>Thanks for putting the code up in the repo.
>>
>>I noticed a few things and wanted to bring them up.
>>
>>I saw all pom files are currently using tabs instead of spaces. Will
>>you be fixing them?
>>
>>And not directly related to the style, but indirectly when the
>>stylecheck gets activated by default and when you want to disable it
>>for a quick build, etc, could you add the fastinstall profile (to skip
>>some steps for a quick build)?
>>
>>+        <profile>
>>+            <id>fastinstall</id>
>>+            <properties>
>>+                <maven.test.skip>true</maven.test.skip>
>>+                <!-- here we could also add checkstyle.skip = true
>>etc once the checkstyle gets activated by default -->
>>+            </properties>
>>+        </profile>
>>
>>and the defaultGoal in build would be nice :-)
>>        <build>
>>+        <defaultGoal>install</defaultGoal>
>>
>>thanks.
>>regards, aki
>>p.s.  by the way, I was testing the code against cxf-2.7.6. Everything
>>runs fine. So you can upgrade its cxf dependency to 2.7.6.
>

Re: code style

Posted by "Bolz, Michael" <mi...@sap.com>.
Hi and Welcome Aki,


The Tabs in the pom will be fixed with the "initial code clean up run".
Which I think I will do till end of the week if nobody calls "veto".

Our other points sounds interesting ("fast install") and should be
discussed.
The checkstyle plugin should only be executed within the "build.quality"
profile.
With other profiles it should be only executed if manually called (e.g.
via "mvn checkstyle:check") because of the "<execution>" configuration.
At least within my environment it works that way. Has anybody another
experience?


Nevertheless I think we can set the default goal to "install".

Kind regards,
Michael

PS: Thanks for the test with cxf-2.7.6  ;o)

On 13.08.13 15:05, "Aki Yoshida" <el...@gmail.com> wrote:

>Hi,
>first time posting here :-)
>Thanks for putting the code up in the repo.
>
>I noticed a few things and wanted to bring them up.
>
>I saw all pom files are currently using tabs instead of spaces. Will
>you be fixing them?
>
>And not directly related to the style, but indirectly when the
>stylecheck gets activated by default and when you want to disable it
>for a quick build, etc, could you add the fastinstall profile (to skip
>some steps for a quick build)?
>
>+        <profile>
>+            <id>fastinstall</id>
>+            <properties>
>+                <maven.test.skip>true</maven.test.skip>
>+                <!-- here we could also add checkstyle.skip = true
>etc once the checkstyle gets activated by default -->
>+            </properties>
>+        </profile>
>
>and the defaultGoal in build would be nice :-)
>        <build>
>+        <defaultGoal>install</defaultGoal>
>
>thanks.
>regards, aki
>p.s.  by the way, I was testing the code against cxf-2.7.6. Everything
>runs fine. So you can upgrade its cxf dependency to 2.7.6.


Re: code style

Posted by Aki Yoshida <el...@gmail.com>.
Hi,
first time posting here :-)
Thanks for putting the code up in the repo.

I noticed a few things and wanted to bring them up.

I saw all pom files are currently using tabs instead of spaces. Will
you be fixing them?

And not directly related to the style, but indirectly when the
stylecheck gets activated by default and when you want to disable it
for a quick build, etc, could you add the fastinstall profile (to skip
some steps for a quick build)?

+        <profile>
+            <id>fastinstall</id>
+            <properties>
+                <maven.test.skip>true</maven.test.skip>
+                <!-- here we could also add checkstyle.skip = true
etc once the checkstyle gets activated by default -->
+            </properties>
+        </profile>

and the defaultGoal in build would be nice :-)
        <build>
+        <defaultGoal>install</defaultGoal>

thanks.
regards, aki
p.s.  by the way, I was testing the code against cxf-2.7.6. Everything
runs fine. So you can upgrade its cxf dependency to 2.7.6.

2013/8/13 Bolz, Michael <mi...@sap.com>:
> Hi,
>
> Weekend is over and the result for line wrapping looks as follows:
> +6: "line wrapping policy"
> +1: "80 chars"
> +3: "120 chars"
> +2: "135 chars"
>
>
> So the winner is "line wrapping=true" with "120 chars".
>
> I adapted the checkstyle configuration and the usage in the parent pom
> (see commit "2849d4eba978b83155c6ac92cc46087134ff1ffe").
> Currently checkstyle has to be run manually (e.g. "mvn checkstyle:check")
> or over the quality build profile ("mvn package -Pbuild.quality").
>
> Next question is how we would "clean up" the code base?
> My suggestion would be to do one single "clean up run and git push"
> without further code changes.
> What do you think?
>
> Kind regards and a nice week
> Michael
>

RE: code style

Posted by "Huesken, Jens" <je...@sap.com>.
Hi Michael,
>> Next question is how we would "clean up" the code base?
>> My suggestion would be to do one single "clean up run and git push" without further code changes.
>> What do you think?

Sounds great! Maybe you can tell us the timeframe when you are planning to do the cleanup to avoid any merge conflicts?

Best regards,
Jens.

-----Original Message-----
From: Bolz, Michael [mailto:michael.bolz@sap.com] 
Sent: Dienstag, 13. August 2013 08:37
To: dev@olingo.incubator.apache.org
Subject: Re: code style

Hi,

Weekend is over and the result for line wrapping looks as follows:
+6: "line wrapping policy"
+1: "80 chars"
+3: "120 chars"
+2: "135 chars"


So the winner is "line wrapping=true" with "120 chars".

I adapted the checkstyle configuration and the usage in the parent pom
(see commit "2849d4eba978b83155c6ac92cc46087134ff1ffe").
Currently checkstyle has to be run manually (e.g. "mvn checkstyle:check")
or over the quality build profile ("mvn package -Pbuild.quality").

Next question is how we would "clean up" the code base?
My suggestion would be to do one single "clean up run and git push"
without further code changes.
What do you think? 

Kind regards and a nice week
Michael


Re: code style

Posted by "Bolz, Michael" <mi...@sap.com>.
Hi,

Weekend is over and the result for line wrapping looks as follows:
+6: "line wrapping policy"
+1: "80 chars"
+3: "120 chars"
+2: "135 chars"


So the winner is "line wrapping=true" with "120 chars".

I adapted the checkstyle configuration and the usage in the parent pom
(see commit "2849d4eba978b83155c6ac92cc46087134ff1ffe").
Currently checkstyle has to be run manually (e.g. "mvn checkstyle:check")
or over the quality build profile ("mvn package -Pbuild.quality").

Next question is how we would "clean up" the code base?
My suggestion would be to do one single "clean up run and git push"
without further code changes.
What do you think? 

Kind regards and a nice week
Michael


Re: code style

Posted by "Bolz, Michael" <mi...@sap.com>.
Hi,

In my opinion we could define a "line wrapping policy". But for me is a
line width of 80 chars much to low.
I think today normally at each monitor (> 19") you can handle ~135 chars
without scrolling.

So for me:
+1: "line wrapping policy"
-1: "80 chars"
+1: "135 chars"

Any other opinions?

Kind regards,
Michael



On 05.08.13 11:49, "V.A, Chandan" <ch...@sap.com> wrote:

>Hi Stephan,
>Can we have line wrapping policy for code-style formatter set to "Wrap
>where necessary" or can we have a defined line width of 80 chars. This
>could avoid scrolling.
>
>Thanks,
>Kind Regards
>Chandan VA
>
>-----Original Message-----
>From: Klevenz, Stephan [mailto:stephan.klevenz@sap.com]
>Sent: Wednesday, July 31, 2013 5:42 PM
>To: dev@olingo.incubator.apache.org
>Subject: code style
>
>Hi,
>
>The original code was implemented by using these code styles for Eclipse:
>
>https://git-wip-us.apache.org/repos/asf?p=incubator-olingo-odata2.git;a=tr
>ee;f=src/eclipse;h=420ec7b716ea24ccd66f4bfddf2c08ccf8c86f6c;hb=HEAD
>
>I suggest to continue using them and do a clean up code before each
>commit. If someone does not agree to this settings then feel free to
>discuss and it is an option to change.
>
>-- Stephan
>
>BTW.: There are currently a lot of javadoc warnings in the build. Maybe
>everyone can have a look into this and get it fixed.