checkstyle improvements

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

checkstyle improvements

Michael Brohl-3
Hi *,

checkstyle currently reports a huge amount of errors. We currently have
an error count setup in the configuration to prevent the build from
failing because of the present errors.

Some thoughts/questions to the community:

* should we take an approach to fine-tune the  configuration so that it
better fits the project style?

As an example, constants are currently not allowed to be named "module",
"resource" etc. which is a common pattern in our code. Changing from
^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
would allow the common naming.

The opposite approach would be to rename those to fit the default settings.

* should we start an initiative to remove the valid errors like we did
with the FindBugs initiative some time ago?


Thanks for your feedback,

Michael Brohl

ecomify GmbH - www.ecomify.de



smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Daniel Watford
Hi Michael,

Improving the checkstyle/findbugs metrics could be a good theme for the
February Community Days as these sorts of fixes are often low risk and you
get a lot of help from your IDE.

These sorts of tasks are often a good way for new developers to get
involved as they don't require in-depth knowledge of the code, as long as
they stick to the principle of only renaming variables/constants, and avoid
too much code refactoring.

The downside is that the quantity of changes to review can be high, and
since many source code files would likely be modified, you would need
reviewers who are able to respond quickly. But we could encourage
developers to try and keep any 'variable rename sets' as small as possible.

Dan.

On Thu, 13 Feb 2020 at 16:44, Michael Brohl <[hidden email]>
wrote:

> Hi *,
>
> checkstyle currently reports a huge amount of errors. We currently have
> an error count setup in the configuration to prevent the build from
> failing because of the present errors.
>
> Some thoughts/questions to the community:
>
> * should we take an approach to fine-tune the  configuration so that it
> better fits the project style?
>
> As an example, constants are currently not allowed to be named "module",
> "resource" etc. which is a common pattern in our code. Changing from
> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> would allow the common naming.
>
> The opposite approach would be to rename those to fit the default settings.
>
> * should we start an initiative to remove the valid errors like we did
> with the FindBugs initiative some time ago?
>
>
> Thanks for your feedback,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
>

--
Daniel Watford
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Jacques Le Roux
Administrator
In reply to this post by Michael Brohl-3
Inline...

Le 13/02/2020 à 17:44, Michael Brohl a écrit :
> Hi *,
>
> checkstyle currently reports a huge amount of errors. We currently have an error count setup in the configuration to prevent the build from failing
> because of the present errors.
>
> Some thoughts/questions to the community:
>
> * should we take an approach to fine-tune the  configuration so that it better fits the project style?

+1, eg I was surprises by checkstyle rejecting {} when { } was OK, sounded a bit harsh to me


>
> As an example, constants are currently not allowed to be named "module", "resource" etc. which is a common pattern in our code. Changing from
> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$ would allow the common naming.
>
> The opposite approach would be to rename those to fit the default settings.
>
> * should we start an initiative to remove the valid errors like we did with the FindBugs initiative some time ago?

+1 and what would be useful is to have a count on the error/s before and after a commit and very useful the list of changes in error.

Thanks

Jacques

Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
In reply to this post by Daniel Watford
Thanks for your feedback, Dan.

Engaging new contributors was one of the positive effects I had in mind too.

If we'd do refactorings per class/package, it should be good to handle.
We had good experience during the FindBugs initiative [1].

Thanks,

Michael Brohl

ecomify GmbH - www.ecomify.de


Am 13.02.20 um 17:55 schrieb Daniel Watford:

> Hi Michael,
>
> Improving the checkstyle/findbugs metrics could be a good theme for the
> February Community Days as these sorts of fixes are often low risk and you
> get a lot of help from your IDE.
>
> These sorts of tasks are often a good way for new developers to get
> involved as they don't require in-depth knowledge of the code, as long as
> they stick to the principle of only renaming variables/constants, and avoid
> too much code refactoring.
>
> The downside is that the quantity of changes to review can be high, and
> since many source code files would likely be modified, you would need
> reviewers who are able to respond quickly. But we could encourage
> developers to try and keep any 'variable rename sets' as small as possible.
>
> Dan.
>
> On Thu, 13 Feb 2020 at 16:44, Michael Brohl <[hidden email]>
> wrote:
>
>> Hi *,
>>
>> checkstyle currently reports a huge amount of errors. We currently have
>> an error count setup in the configuration to prevent the build from
>> failing because of the present errors.
>>
>> Some thoughts/questions to the community:
>>
>> * should we take an approach to fine-tune the  configuration so that it
>> better fits the project style?
>>
>> As an example, constants are currently not allowed to be named "module",
>> "resource" etc. which is a common pattern in our code. Changing from
>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
>> would allow the common naming.
>>
>> The opposite approach would be to rename those to fit the default settings.
>>
>> * should we start an initiative to remove the valid errors like we did
>> with the FindBugs initiative some time ago?
>>
>>
>> Thanks for your feedback,
>>
>> Michael Brohl
>>
>> ecomify GmbH - www.ecomify.de
>>
>>
>>


smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
Hi *, and others (real people included),

The project's representations in SonarCloud.io (as a free service via the
ASF) gives us up-to-date information on a variety of issues related to the
various types of code (java, groovy, js, xml, etc.), like:

   - bugs
   - security hotspots
   - code smells
   - duplicate blocks

See https://sonarcloud.io/dashboard?id=apache_ofbiz-framework and
https://sonarcloud.io/dashboard?id=apache_ofbiz-plugins. With a PullRequest
submitted to the Github repository we see in comments thereon how the
commits are assessed. See e.g.
https://github.com/apache/ofbiz-framework/pull/14

Best regards,

Pierre Smits
*Proud* *contributor* (but without privileges)* of* Apache OFBiz
<https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer


On Thu, Feb 13, 2020 at 6:04 PM Michael Brohl <[hidden email]>
wrote:

> Thanks for your feedback, Dan.
>
> Engaging new contributors was one of the positive effects I had in mind
> too.
>
> If we'd do refactorings per class/package, it should be good to handle.
> We had good experience during the FindBugs initiative [1].
>
> Thanks,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
> Am 13.02.20 um 17:55 schrieb Daniel Watford:
> > Hi Michael,
> >
> > Improving the checkstyle/findbugs metrics could be a good theme for the
> > February Community Days as these sorts of fixes are often low risk and
> you
> > get a lot of help from your IDE.
> >
> > These sorts of tasks are often a good way for new developers to get
> > involved as they don't require in-depth knowledge of the code, as long as
> > they stick to the principle of only renaming variables/constants, and
> avoid
> > too much code refactoring.
> >
> > The downside is that the quantity of changes to review can be high, and
> > since many source code files would likely be modified, you would need
> > reviewers who are able to respond quickly. But we could encourage
> > developers to try and keep any 'variable rename sets' as small as
> possible.
> >
> > Dan.
> >
> > On Thu, 13 Feb 2020 at 16:44, Michael Brohl <[hidden email]>
> > wrote:
> >
> >> Hi *,
> >>
> >> checkstyle currently reports a huge amount of errors. We currently have
> >> an error count setup in the configuration to prevent the build from
> >> failing because of the present errors.
> >>
> >> Some thoughts/questions to the community:
> >>
> >> * should we take an approach to fine-tune the  configuration so that it
> >> better fits the project style?
> >>
> >> As an example, constants are currently not allowed to be named "module",
> >> "resource" etc. which is a common pattern in our code. Changing from
> >> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> >> would allow the common naming.
> >>
> >> The opposite approach would be to rename those to fit the default
> settings.
> >>
> >> * should we start an initiative to remove the valid errors like we did
> >> with the FindBugs initiative some time ago?
> >>
> >>
> >> Thanks for your feedback,
> >>
> >> Michael Brohl
> >>
> >> ecomify GmbH - www.ecomify.de
> >>
> >>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Gil Portenseigne
In reply to this post by Michael Brohl-3
Hello Michael,

Adapting checkstyle configuration is less impacting to our codebase but
make us stay different from the java standard.
That is the easier path, that will not affect code history.

But about getting nearer from the java standard is IMO a nice to have, to
make pure java developer feel better discovering OFBiz.
I'm kinda prefer the "opposite approach", but we need to discuss if this
improvement is worth the history lost.

In the example you chose, i see no issue capitalizing module,
resource and other. Updating the rule offer the ability to write a
constant like : MoDuLe  :-).

Lots of errors are about line that are 120+ length, missing or uneeded
space etc.

So that will lead to loads of unrisky modifications. With the usage of
IDE with checkstyle plugin this can be done nicely.
I check that over the near thousand of java file concerned more than 80%
have less than 50 issue... So we can focus our effort onto the big files
using a branch to test the idea and ease the issue.

And like Daniel said, it is a good subject for a community week effort.

I'd really like to be able to efficiently use this feature, in one way
or the other, so thank you for starting this discussion.

Regards,

Gil


On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:

> Hi *,
>
> checkstyle currently reports a huge amount of errors. We currently have an
> error count setup in the configuration to prevent the build from failing
> because of the present errors.
>
> Some thoughts/questions to the community:
>
> * should we take an approach to fine-tune the  configuration so that it
> better fits the project style?
>
> As an example, constants are currently not allowed to be named "module",
> "resource" etc. which is a common pattern in our code. Changing from
> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> would allow the common naming.
>
> The opposite approach would be to rename those to fit the default settings.
>
> * should we start an initiative to remove the valid errors like we did with
> the FindBugs initiative some time ago?
>
>
> Thanks for your feedback,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
Hi Gil,

thanks for your thoughts. Inline...


Am 13.02.20 um 21:12 schrieb Gil Portenseigne:

> Hello Michael,
>
> Adapting checkstyle configuration is less impacting to our codebase but
> make us stay different from the java standard.
> That is the easier path, that will not affect code history.
>
> But about getting nearer from the java standard is IMO a nice to have, to
> make pure java developer feel better discovering OFBiz.
> I'm kinda prefer the "opposite approach", but we need to discuss if this
> improvement is worth the history lost.
>
> In the example you chose, i see no issue capitalizing module,
> resource and other. Updating the rule offer the ability to write a
> constant like : MoDuLe  :-).
So you are suggesting to use the default configuration for constant
names and do a refactoring to make the private constants like module,
ressource, ressource_error follow this configuration (capitalized with
underscores)? This would result lots of modifications.

I've also seen a configuration option apply the pattern only to
public/protected constants which could be an alternative to reduce the
amount of changes, at least in the first iterations.

I am open for these alternatives, as long as we can manage to get it
into the codebase smoothly.


>
> Lots of errors are about line that are 120+ length, missing or uneeded
> space etc.
>
> So that will lead to loads of unrisky modifications. With the usage of
> IDE with checkstyle plugin this can be done nicely.

I've seen several plugins for Eclipse, any suggestions of a plugin which
is able to use our configuration?


> I check that over the near thousand of java file concerned more than 80%
> have less than 50 issue... So we can focus our effort onto the big files
> using a branch to test the idea and ease the issue.
>
> And like Daniel said, it is a good subject for a community week effort.

+1

To make this work out and keep it manageable, we should define some
"rules" how to approach this.

For example, I'm thinking of

- a clear separation of checkstyle modifications and other code
corrections which might come to mind when reading the code

- checkstyle modifications should not change the functionality/program
code in any way. Which means that open API should not be changed either
(e.g. public constants that might be used in third party functionality).
If we want to change them, we should deprecate the old and remove in the
upcoming release branch. So this needs some caution.

- formatting only where it is necessary (no reformatting of the whole file)

- ...

This would make it easier for committers to review and accept the changes.

If IDE plugins are used, they should be able to read the
config/checkstyle.xml to use the same configuration for every check.


>
> I'd really like to be able to efficiently use this feature, in one way
> or the other, so thank you for starting this discussion.
>
> Regards,
>
> Gil


Thanks Gil,

Michael Brohl

ecomify GmbH - www.ecomify.de




smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
In reply to this post by Gil Portenseigne
It seems many projects (including those Apache projects in the Hadoop
sphere) use the works of the Apache Yetus (see [1]) project to do
pre-commit checks (including IIUC checkstyle, in an automate way) on patch
files attached to JIRA tickets and Pull Requests in Github against the
latest commit in the branch. This may be helpful to lessen the burden on
the contributor. When configured correctly, the test results appear in the
ticket. Apache HBASE is such a project using Yetus, See as an example [2]
where test results are included through comments of 'Hadoop QA'.

Maybe we should consider this for our project?


[1] https://yetus.apache.org
[2]
https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC

Best regards,

Pierre Smits
*Proud* *contributor* (but without privileges)* of* Apache OFBiz
<https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer


On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
[hidden email]> wrote:

> Hello Michael,
>
> Adapting checkstyle configuration is less impacting to our codebase but
> make us stay different from the java standard.
> That is the easier path, that will not affect code history.
>
> But about getting nearer from the java standard is IMO a nice to have, to
> make pure java developer feel better discovering OFBiz.
> I'm kinda prefer the "opposite approach", but we need to discuss if this
> improvement is worth the history lost.
>
> In the example you chose, i see no issue capitalizing module,
> resource and other. Updating the rule offer the ability to write a
> constant like : MoDuLe  :-).
>
> Lots of errors are about line that are 120+ length, missing or uneeded
> space etc.
>
> So that will lead to loads of unrisky modifications. With the usage of
> IDE with checkstyle plugin this can be done nicely.
> I check that over the near thousand of java file concerned more than 80%
> have less than 50 issue... So we can focus our effort onto the big files
> using a branch to test the idea and ease the issue.
>
> And like Daniel said, it is a good subject for a community week effort.
>
> I'd really like to be able to efficiently use this feature, in one way
> or the other, so thank you for starting this discussion.
>
> Regards,
>
> Gil
>
>
> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
> > Hi *,
> >
> > checkstyle currently reports a huge amount of errors. We currently have
> an
> > error count setup in the configuration to prevent the build from failing
> > because of the present errors.
> >
> > Some thoughts/questions to the community:
> >
> > * should we take an approach to fine-tune the  configuration so that it
> > better fits the project style?
> >
> > As an example, constants are currently not allowed to be named "module",
> > "resource" etc. which is a common pattern in our code. Changing from
> > ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> > would allow the common naming.
> >
> > The opposite approach would be to rename those to fit the default
> settings.
> >
> > * should we start an initiative to remove the valid errors like we did
> with
> > the FindBugs initiative some time ago?
> >
> >
> > Thanks for your feedback,
> >
> > Michael Brohl
> >
> > ecomify GmbH - www.ecomify.de
> >
> >
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Gil Portenseigne
Hello Pierre,

It is interesting but i guess that will be the next step.

Thanks

On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:

> It seems many projects (including those Apache projects in the Hadoop
> sphere) use the works of the Apache Yetus (see [1]) project to do
> pre-commit checks (including IIUC checkstyle, in an automate way) on patch
> files attached to JIRA tickets and Pull Requests in Github against the
> latest commit in the branch. This may be helpful to lessen the burden on
> the contributor. When configured correctly, the test results appear in the
> ticket. Apache HBASE is such a project using Yetus, See as an example [2]
> where test results are included through comments of 'Hadoop QA'.
>
> Maybe we should consider this for our project?
>
>
> [1] https://yetus.apache.org
> [2]
> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
>
> Best regards,
>
> Pierre Smits
> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
> <https://ofbiz.apache.org/>, since 2008
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>
>
> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
> [hidden email]> wrote:
>
> > Hello Michael,
> >
> > Adapting checkstyle configuration is less impacting to our codebase but
> > make us stay different from the java standard.
> > That is the easier path, that will not affect code history.
> >
> > But about getting nearer from the java standard is IMO a nice to have, to
> > make pure java developer feel better discovering OFBiz.
> > I'm kinda prefer the "opposite approach", but we need to discuss if this
> > improvement is worth the history lost.
> >
> > In the example you chose, i see no issue capitalizing module,
> > resource and other. Updating the rule offer the ability to write a
> > constant like : MoDuLe  :-).
> >
> > Lots of errors are about line that are 120+ length, missing or uneeded
> > space etc.
> >
> > So that will lead to loads of unrisky modifications. With the usage of
> > IDE with checkstyle plugin this can be done nicely.
> > I check that over the near thousand of java file concerned more than 80%
> > have less than 50 issue... So we can focus our effort onto the big files
> > using a branch to test the idea and ease the issue.
> >
> > And like Daniel said, it is a good subject for a community week effort.
> >
> > I'd really like to be able to efficiently use this feature, in one way
> > or the other, so thank you for starting this discussion.
> >
> > Regards,
> >
> > Gil
> >
> >
> > On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
> > > Hi *,
> > >
> > > checkstyle currently reports a huge amount of errors. We currently have
> > an
> > > error count setup in the configuration to prevent the build from failing
> > > because of the present errors.
> > >
> > > Some thoughts/questions to the community:
> > >
> > > * should we take an approach to fine-tune the  configuration so that it
> > > better fits the project style?
> > >
> > > As an example, constants are currently not allowed to be named "module",
> > > "resource" etc. which is a common pattern in our code. Changing from
> > > ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> > > would allow the common naming.
> > >
> > > The opposite approach would be to rename those to fit the default
> > settings.
> > >
> > > * should we start an initiative to remove the valid errors like we did
> > with
> > > the FindBugs initiative some time ago?
> > >
> > >
> > > Thanks for your feedback,
> > >
> > > Michael Brohl
> > >
> > > ecomify GmbH - www.ecomify.de
> > >
> > >
> >
> >
> >

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Gil Portenseigne
In reply to this post by Michael Brohl-3
Inline,

> > I'm kinda prefer the "opposite approach", but we need to discuss if this
> > improvement is worth the history lost.
> >
> > In the example you chose, i see no issue capitalizing module,
> > resource and other. Updating the rule offer the ability to write a
> > constant like : MoDuLe  :-).
>
> So you are suggesting to use the default configuration for constant names
> and do a refactoring to make the private constants like module, ressource,
> ressource_error follow this configuration (capitalized with underscores)?
> This would result lots of modifications.
I agree, and those because having a rule that do not protect against
strange code, seems not a good fit for Checkstyle.
>
> I've also seen a configuration option apply the pattern only to
> public/protected constants which could be an alternative to reduce the
> amount of changes, at least in the first iterations.
>
> I am open for these alternatives, as long as we can manage to get it into
> the codebase smoothly.
Yes that is an option.

>
>
> >
> > Lots of errors are about line that are 120+ length, missing or uneeded
> > space etc.
> >
> > So that will lead to loads of unrisky modifications. With the usage of
> > IDE with checkstyle plugin this can be done nicely.
>
> I've seen several plugins for Eclipse, any suggestions of a plugin which is
> able to use our configuration?
I got idea Checkstyle plugin (CheckStyle-Idea) to work with a slight
modification of our configuration, moving "LineLength" module under
"Checker" module [1]

And got it working nicely with the same result as gradle task.
But i'm pretty sure that Eclipse ones will do the job.

But this modification break the gradle task... so we need to clear that
out.

[...]

> >
> > And like Daniel said, it is a good subject for a community week effort.
>
> +1
>
> To make this work out and keep it manageable, we should define some "rules"
> how to approach this.
>
> For example, I'm thinking of
>
> - a clear separation of checkstyle modifications and other code corrections
> which might come to mind when reading the code
+1
>
> - checkstyle modifications should not change the functionality/program code
> in any way. Which means that open API should not be changed either (e.g.
> public constants that might be used in third party functionality). If we
> want to change them, we should deprecate the old and remove in the upcoming
> release branch. So this needs some caution.

That's the hard part.
>
> - formatting only where it is necessary (no reformatting of the whole file)
+1

Thanks

[1] https://github.com/checkstyle/eclipse-cs/issues/190

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
In reply to this post by Gil Portenseigne
Hi Gil, all,

I agree. Something like that should not be adopted and incorporated in our
vetting processes without a Proof of Concept.

Having said that, preliminary results in a local setup look promising.

Met vriendelijke groet,


Pierre

Op vr 14 feb. 2020 16:34 schreef Gil Portenseigne <
[hidden email]>:

> Hello Pierre,
>
> It is interesting but i guess that will be the next step.
>
> Thanks
>
> On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
> > It seems many projects (including those Apache projects in the Hadoop
> > sphere) use the works of the Apache Yetus (see [1]) project to do
> > pre-commit checks (including IIUC checkstyle, in an automate way) on
> patch
> > files attached to JIRA tickets and Pull Requests in Github against the
> > latest commit in the branch. This may be helpful to lessen the burden on
> > the contributor. When configured correctly, the test results appear in
> the
> > ticket. Apache HBASE is such a project using Yetus, See as an example [2]
> > where test results are included through comments of 'Hadoop QA'.
> >
> > Maybe we should consider this for our project?
> >
> >
> > [1] https://yetus.apache.org
> > [2]
> >
> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
> >
> > Best regards,
> >
> > Pierre Smits
> > *Proud* *contributor* (but without privileges)* of* Apache OFBiz
> > <https://ofbiz.apache.org/>, since 2008
> >
> > *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> > *Apache Directory <https://directory.apache.org>, PMC Member*
> > Apache Incubator <https://incubator.apache.org>, committer
> > Apache Steve <https://steve.apache.org>, committer
> >
> >
> > On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
> > [hidden email]> wrote:
> >
> > > Hello Michael,
> > >
> > > Adapting checkstyle configuration is less impacting to our codebase but
> > > make us stay different from the java standard.
> > > That is the easier path, that will not affect code history.
> > >
> > > But about getting nearer from the java standard is IMO a nice to have,
> to
> > > make pure java developer feel better discovering OFBiz.
> > > I'm kinda prefer the "opposite approach", but we need to discuss if
> this
> > > improvement is worth the history lost.
> > >
> > > In the example you chose, i see no issue capitalizing module,
> > > resource and other. Updating the rule offer the ability to write a
> > > constant like : MoDuLe  :-).
> > >
> > > Lots of errors are about line that are 120+ length, missing or uneeded
> > > space etc.
> > >
> > > So that will lead to loads of unrisky modifications. With the usage of
> > > IDE with checkstyle plugin this can be done nicely.
> > > I check that over the near thousand of java file concerned more than
> 80%
> > > have less than 50 issue... So we can focus our effort onto the big
> files
> > > using a branch to test the idea and ease the issue.
> > >
> > > And like Daniel said, it is a good subject for a community week effort.
> > >
> > > I'd really like to be able to efficiently use this feature, in one way
> > > or the other, so thank you for starting this discussion.
> > >
> > > Regards,
> > >
> > > Gil
> > >
> > >
> > > On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
> > > > Hi *,
> > > >
> > > > checkstyle currently reports a huge amount of errors. We currently
> have
> > > an
> > > > error count setup in the configuration to prevent the build from
> failing
> > > > because of the present errors.
> > > >
> > > > Some thoughts/questions to the community:
> > > >
> > > > * should we take an approach to fine-tune the  configuration so that
> it
> > > > better fits the project style?
> > > >
> > > > As an example, constants are currently not allowed to be named
> "module",
> > > > "resource" etc. which is a common pattern in our code. Changing from
> > > > ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to
> ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> > > > would allow the common naming.
> > > >
> > > > The opposite approach would be to rename those to fit the default
> > > settings.
> > > >
> > > > * should we start an initiative to remove the valid errors like we
> did
> > > with
> > > > the FindBugs initiative some time ago?
> > > >
> > > >
> > > > Thanks for your feedback,
> > > >
> > > > Michael Brohl
> > > >
> > > > ecomify GmbH - www.ecomify.de
> > > >
> > > >
> > >
> > >
> > >
>
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
In reply to this post by Gil Portenseigne
+1 Gil

Pierre, can you please stop cluttering the issues with the copy/pasted
yetus artifacts from your local environment?

At least until we decided to use it and have a setup on the official
infrastructure.

Thanks,

Michael


Am 14.02.20 um 16:34 schrieb Gil Portenseigne:

> Hello Pierre,
>
> It is interesting but i guess that will be the next step.
>
> Thanks
>
> On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
>> It seems many projects (including those Apache projects in the Hadoop
>> sphere) use the works of the Apache Yetus (see [1]) project to do
>> pre-commit checks (including IIUC checkstyle, in an automate way) on patch
>> files attached to JIRA tickets and Pull Requests in Github against the
>> latest commit in the branch. This may be helpful to lessen the burden on
>> the contributor. When configured correctly, the test results appear in the
>> ticket. Apache HBASE is such a project using Yetus, See as an example [2]
>> where test results are included through comments of 'Hadoop QA'.
>>
>> Maybe we should consider this for our project?
>>
>>
>> [1] https://yetus.apache.org
>> [2]
>> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
>>
>> Best regards,
>>
>> Pierre Smits
>> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
>> <https://ofbiz.apache.org/>, since 2008
>>
>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
>> *Apache Directory <https://directory.apache.org>, PMC Member*
>> Apache Incubator <https://incubator.apache.org>, committer
>> Apache Steve <https://steve.apache.org>, committer
>>
>>
>> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
>> [hidden email]> wrote:
>>
>>> Hello Michael,
>>>
>>> Adapting checkstyle configuration is less impacting to our codebase but
>>> make us stay different from the java standard.
>>> That is the easier path, that will not affect code history.
>>>
>>> But about getting nearer from the java standard is IMO a nice to have, to
>>> make pure java developer feel better discovering OFBiz.
>>> I'm kinda prefer the "opposite approach", but we need to discuss if this
>>> improvement is worth the history lost.
>>>
>>> In the example you chose, i see no issue capitalizing module,
>>> resource and other. Updating the rule offer the ability to write a
>>> constant like : MoDuLe  :-).
>>>
>>> Lots of errors are about line that are 120+ length, missing or uneeded
>>> space etc.
>>>
>>> So that will lead to loads of unrisky modifications. With the usage of
>>> IDE with checkstyle plugin this can be done nicely.
>>> I check that over the near thousand of java file concerned more than 80%
>>> have less than 50 issue... So we can focus our effort onto the big files
>>> using a branch to test the idea and ease the issue.
>>>
>>> And like Daniel said, it is a good subject for a community week effort.
>>>
>>> I'd really like to be able to efficiently use this feature, in one way
>>> or the other, so thank you for starting this discussion.
>>>
>>> Regards,
>>>
>>> Gil
>>>
>>>
>>> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
>>>> Hi *,
>>>>
>>>> checkstyle currently reports a huge amount of errors. We currently have
>>> an
>>>> error count setup in the configuration to prevent the build from failing
>>>> because of the present errors.
>>>>
>>>> Some thoughts/questions to the community:
>>>>
>>>> * should we take an approach to fine-tune the  configuration so that it
>>>> better fits the project style?
>>>>
>>>> As an example, constants are currently not allowed to be named "module",
>>>> "resource" etc. which is a common pattern in our code. Changing from
>>>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
>>>> would allow the common naming.
>>>>
>>>> The opposite approach would be to rename those to fit the default
>>> settings.
>>>> * should we start an initiative to remove the valid errors like we did
>>> with
>>>> the FindBugs initiative some time ago?
>>>>
>>>>
>>>> Thanks for your feedback,
>>>>
>>>> Michael Brohl
>>>>
>>>> ecomify GmbH - www.ecomify.de
>>>>
>>>>
>>>
>>>


smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
Michael,

You mean like:

   - *stop* posting comments to tickets to help fellow contributors and the
   adopters of our product looking to these contributions to help them decide
   whether or not to invest in an adoption/implementation of (or looking for
   guidance on issues they experience with their implementation of) the
   product?
   - *stop* posting to mail threads to provide insight and such,?
   - *stop* providing reactions to comments posted to pages in our wiki?
   - *stop* taking the initiative?

Met vriendelijke groet,

Pierre Smits
*Proud* *contributor* (but unfortunately without privileges)* of* Apache
OFBiz <https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer


On Wed, Feb 19, 2020 at 3:05 PM Michael Brohl <[hidden email]>
wrote:

> +1 Gil
>
> Pierre, can you please stop cluttering the issues with the copy/pasted
> yetus artifacts from your local environment?
>
> At least until we decided to use it and have a setup on the official
> infrastructure.
>
> Thanks,
>
> Michael
>
>
> Am 14.02.20 um 16:34 schrieb Gil Portenseigne:
> > Hello Pierre,
> >
> > It is interesting but i guess that will be the next step.
> >
> > Thanks
> >
> > On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
> >> It seems many projects (including those Apache projects in the Hadoop
> >> sphere) use the works of the Apache Yetus (see [1]) project to do
> >> pre-commit checks (including IIUC checkstyle, in an automate way) on
> patch
> >> files attached to JIRA tickets and Pull Requests in Github against the
> >> latest commit in the branch. This may be helpful to lessen the burden on
> >> the contributor. When configured correctly, the test results appear in
> the
> >> ticket. Apache HBASE is such a project using Yetus, See as an example
> [2]
> >> where test results are included through comments of 'Hadoop QA'.
> >>
> >> Maybe we should consider this for our project?
> >>
> >>
> >> [1] https://yetus.apache.org
> >> [2]
> >>
> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
> >>
> >> Best regards,
> >>
> >> Pierre Smits
> >> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
> >> <https://ofbiz.apache.org/>, since 2008
> >>
> >> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> >> *Apache Directory <https://directory.apache.org>, PMC Member*
> >> Apache Incubator <https://incubator.apache.org>, committer
> >> Apache Steve <https://steve.apache.org>, committer
> >>
> >>
> >> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
> >> [hidden email]> wrote:
> >>
> >>> Hello Michael,
> >>>
> >>> Adapting checkstyle configuration is less impacting to our codebase but
> >>> make us stay different from the java standard.
> >>> That is the easier path, that will not affect code history.
> >>>
> >>> But about getting nearer from the java standard is IMO a nice to have,
> to
> >>> make pure java developer feel better discovering OFBiz.
> >>> I'm kinda prefer the "opposite approach", but we need to discuss if
> this
> >>> improvement is worth the history lost.
> >>>
> >>> In the example you chose, i see no issue capitalizing module,
> >>> resource and other. Updating the rule offer the ability to write a
> >>> constant like : MoDuLe  :-).
> >>>
> >>> Lots of errors are about line that are 120+ length, missing or uneeded
> >>> space etc.
> >>>
> >>> So that will lead to loads of unrisky modifications. With the usage of
> >>> IDE with checkstyle plugin this can be done nicely.
> >>> I check that over the near thousand of java file concerned more than
> 80%
> >>> have less than 50 issue... So we can focus our effort onto the big
> files
> >>> using a branch to test the idea and ease the issue.
> >>>
> >>> And like Daniel said, it is a good subject for a community week effort.
> >>>
> >>> I'd really like to be able to efficiently use this feature, in one way
> >>> or the other, so thank you for starting this discussion.
> >>>
> >>> Regards,
> >>>
> >>> Gil
> >>>
> >>>
> >>> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
> >>>> Hi *,
> >>>>
> >>>> checkstyle currently reports a huge amount of errors. We currently
> have
> >>> an
> >>>> error count setup in the configuration to prevent the build from
> failing
> >>>> because of the present errors.
> >>>>
> >>>> Some thoughts/questions to the community:
> >>>>
> >>>> * should we take an approach to fine-tune the  configuration so that
> it
> >>>> better fits the project style?
> >>>>
> >>>> As an example, constants are currently not allowed to be named
> "module",
> >>>> "resource" etc. which is a common pattern in our code. Changing from
> >>>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to
> ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> >>>> would allow the common naming.
> >>>>
> >>>> The opposite approach would be to rename those to fit the default
> >>> settings.
> >>>> * should we start an initiative to remove the valid errors like we did
> >>> with
> >>>> the FindBugs initiative some time ago?
> >>>>
> >>>>
> >>>> Thanks for your feedback,
> >>>>
> >>>> Michael Brohl
> >>>>
> >>>> ecomify GmbH - www.ecomify.de
> >>>>
> >>>>
> >>>
> >>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
No, I did not say or mean anything like that. Not sure where you get
that from.

Please read again, thanks,

Michael

Am 19.02.20 um 15:51 schrieb Pierre Smits:

> Michael,
>
> You mean like:
>
>     - *stop* posting comments to tickets to help fellow contributors and the
>     adopters of our product looking to these contributions to help them decide
>     whether or not to invest in an adoption/implementation of (or looking for
>     guidance on issues they experience with their implementation of) the
>     product?
>     - *stop* posting to mail threads to provide insight and such,?
>     - *stop* providing reactions to comments posted to pages in our wiki?
>     - *stop* taking the initiative?
>
> Met vriendelijke groet,
>
> Pierre Smits
> *Proud* *contributor* (but unfortunately without privileges)* of* Apache
> OFBiz <https://ofbiz.apache.org/>, since 2008
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>
>
> On Wed, Feb 19, 2020 at 3:05 PM Michael Brohl <[hidden email]>
> wrote:
>
>> +1 Gil
>>
>> Pierre, can you please stop cluttering the issues with the copy/pasted
>> yetus artifacts from your local environment?
>>
>> At least until we decided to use it and have a setup on the official
>> infrastructure.
>>
>> Thanks,
>>
>> Michael
>>
>>
>> Am 14.02.20 um 16:34 schrieb Gil Portenseigne:
>>> Hello Pierre,
>>>
>>> It is interesting but i guess that will be the next step.
>>>
>>> Thanks
>>>
>>> On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
>>>> It seems many projects (including those Apache projects in the Hadoop
>>>> sphere) use the works of the Apache Yetus (see [1]) project to do
>>>> pre-commit checks (including IIUC checkstyle, in an automate way) on
>> patch
>>>> files attached to JIRA tickets and Pull Requests in Github against the
>>>> latest commit in the branch. This may be helpful to lessen the burden on
>>>> the contributor. When configured correctly, the test results appear in
>> the
>>>> ticket. Apache HBASE is such a project using Yetus, See as an example
>> [2]
>>>> where test results are included through comments of 'Hadoop QA'.
>>>>
>>>> Maybe we should consider this for our project?
>>>>
>>>>
>>>> [1] https://yetus.apache.org
>>>> [2]
>>>>
>> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
>>>> Best regards,
>>>>
>>>> Pierre Smits
>>>> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
>>>> <https://ofbiz.apache.org/>, since 2008
>>>>
>>>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
>>>> *Apache Directory <https://directory.apache.org>, PMC Member*
>>>> Apache Incubator <https://incubator.apache.org>, committer
>>>> Apache Steve <https://steve.apache.org>, committer
>>>>
>>>>
>>>> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
>>>> [hidden email]> wrote:
>>>>
>>>>> Hello Michael,
>>>>>
>>>>> Adapting checkstyle configuration is less impacting to our codebase but
>>>>> make us stay different from the java standard.
>>>>> That is the easier path, that will not affect code history.
>>>>>
>>>>> But about getting nearer from the java standard is IMO a nice to have,
>> to
>>>>> make pure java developer feel better discovering OFBiz.
>>>>> I'm kinda prefer the "opposite approach", but we need to discuss if
>> this
>>>>> improvement is worth the history lost.
>>>>>
>>>>> In the example you chose, i see no issue capitalizing module,
>>>>> resource and other. Updating the rule offer the ability to write a
>>>>> constant like : MoDuLe  :-).
>>>>>
>>>>> Lots of errors are about line that are 120+ length, missing or uneeded
>>>>> space etc.
>>>>>
>>>>> So that will lead to loads of unrisky modifications. With the usage of
>>>>> IDE with checkstyle plugin this can be done nicely.
>>>>> I check that over the near thousand of java file concerned more than
>> 80%
>>>>> have less than 50 issue... So we can focus our effort onto the big
>> files
>>>>> using a branch to test the idea and ease the issue.
>>>>>
>>>>> And like Daniel said, it is a good subject for a community week effort.
>>>>>
>>>>> I'd really like to be able to efficiently use this feature, in one way
>>>>> or the other, so thank you for starting this discussion.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Gil
>>>>>
>>>>>
>>>>> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
>>>>>> Hi *,
>>>>>>
>>>>>> checkstyle currently reports a huge amount of errors. We currently
>> have
>>>>> an
>>>>>> error count setup in the configuration to prevent the build from
>> failing
>>>>>> because of the present errors.
>>>>>>
>>>>>> Some thoughts/questions to the community:
>>>>>>
>>>>>> * should we take an approach to fine-tune the  configuration so that
>> it
>>>>>> better fits the project style?
>>>>>>
>>>>>> As an example, constants are currently not allowed to be named
>> "module",
>>>>>> "resource" etc. which is a common pattern in our code. Changing from
>>>>>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to
>> ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
>>>>>> would allow the common naming.
>>>>>>
>>>>>> The opposite approach would be to rename those to fit the default
>>>>> settings.
>>>>>> * should we start an initiative to remove the valid errors like we did
>>>>> with
>>>>>> the FindBugs initiative some time ago?
>>>>>>
>>>>>>
>>>>>> Thanks for your feedback,
>>>>>>
>>>>>> Michael Brohl
>>>>>>
>>>>>> ecomify GmbH - www.ecomify.de
>>>>>>
>>>>>>
>>>>>
>>


smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
What you find cluttering, I regards as helpful to:

   - fellow contributors
   - our (potential) adopters
   - anyone else working on an OFBiz implementation


Met vriendelijke groet,

Pierre Smits
*Proud* *contributor* (but without privileges)* of* Apache OFBiz
<https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer


On Wed, Feb 19, 2020 at 3:54 PM Michael Brohl <[hidden email]>
wrote:

> No, I did not say or mean anything like that. Not sure where you get
> that from.
>
> Please read again, thanks,
>
> Michael
>
> Am 19.02.20 um 15:51 schrieb Pierre Smits:
> > Michael,
> >
> > You mean like:
> >
> >     - *stop* posting comments to tickets to help fellow contributors and
> the
> >     adopters of our product looking to these contributions to help them
> decide
> >     whether or not to invest in an adoption/implementation of (or
> looking for
> >     guidance on issues they experience with their implementation of) the
> >     product?
> >     - *stop* posting to mail threads to provide insight and such,?
> >     - *stop* providing reactions to comments posted to pages in our wiki?
> >     - *stop* taking the initiative?
> >
> > Met vriendelijke groet,
> >
> > Pierre Smits
> > *Proud* *contributor* (but unfortunately without privileges)* of* Apache
> > OFBiz <https://ofbiz.apache.org/>, since 2008
> >
> > *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> > *Apache Directory <https://directory.apache.org>, PMC Member*
> > Apache Incubator <https://incubator.apache.org>, committer
> > Apache Steve <https://steve.apache.org>, committer
> >
> >
> > On Wed, Feb 19, 2020 at 3:05 PM Michael Brohl <[hidden email]>
> > wrote:
> >
> >> +1 Gil
> >>
> >> Pierre, can you please stop cluttering the issues with the copy/pasted
> >> yetus artifacts from your local environment?
> >>
> >> At least until we decided to use it and have a setup on the official
> >> infrastructure.
> >>
> >> Thanks,
> >>
> >> Michael
> >>
> >>
> >> Am 14.02.20 um 16:34 schrieb Gil Portenseigne:
> >>> Hello Pierre,
> >>>
> >>> It is interesting but i guess that will be the next step.
> >>>
> >>> Thanks
> >>>
> >>> On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
> >>>> It seems many projects (including those Apache projects in the Hadoop
> >>>> sphere) use the works of the Apache Yetus (see [1]) project to do
> >>>> pre-commit checks (including IIUC checkstyle, in an automate way) on
> >> patch
> >>>> files attached to JIRA tickets and Pull Requests in Github against the
> >>>> latest commit in the branch. This may be helpful to lessen the burden
> on
> >>>> the contributor. When configured correctly, the test results appear in
> >> the
> >>>> ticket. Apache HBASE is such a project using Yetus, See as an example
> >> [2]
> >>>> where test results are included through comments of 'Hadoop QA'.
> >>>>
> >>>> Maybe we should consider this for our project?
> >>>>
> >>>>
> >>>> [1] https://yetus.apache.org
> >>>> [2]
> >>>>
> >>
> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
> >>>> Best regards,
> >>>>
> >>>> Pierre Smits
> >>>> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
> >>>> <https://ofbiz.apache.org/>, since 2008
> >>>>
> >>>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> >>>> *Apache Directory <https://directory.apache.org>, PMC Member*
> >>>> Apache Incubator <https://incubator.apache.org>, committer
> >>>> Apache Steve <https://steve.apache.org>, committer
> >>>>
> >>>>
> >>>> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
> >>>> [hidden email]> wrote:
> >>>>
> >>>>> Hello Michael,
> >>>>>
> >>>>> Adapting checkstyle configuration is less impacting to our codebase
> but
> >>>>> make us stay different from the java standard.
> >>>>> That is the easier path, that will not affect code history.
> >>>>>
> >>>>> But about getting nearer from the java standard is IMO a nice to
> have,
> >> to
> >>>>> make pure java developer feel better discovering OFBiz.
> >>>>> I'm kinda prefer the "opposite approach", but we need to discuss if
> >> this
> >>>>> improvement is worth the history lost.
> >>>>>
> >>>>> In the example you chose, i see no issue capitalizing module,
> >>>>> resource and other. Updating the rule offer the ability to write a
> >>>>> constant like : MoDuLe  :-).
> >>>>>
> >>>>> Lots of errors are about line that are 120+ length, missing or
> uneeded
> >>>>> space etc.
> >>>>>
> >>>>> So that will lead to loads of unrisky modifications. With the usage
> of
> >>>>> IDE with checkstyle plugin this can be done nicely.
> >>>>> I check that over the near thousand of java file concerned more than
> >> 80%
> >>>>> have less than 50 issue... So we can focus our effort onto the big
> >> files
> >>>>> using a branch to test the idea and ease the issue.
> >>>>>
> >>>>> And like Daniel said, it is a good subject for a community week
> effort.
> >>>>>
> >>>>> I'd really like to be able to efficiently use this feature, in one
> way
> >>>>> or the other, so thank you for starting this discussion.
> >>>>>
> >>>>> Regards,
> >>>>>
> >>>>> Gil
> >>>>>
> >>>>>
> >>>>> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
> >>>>>> Hi *,
> >>>>>>
> >>>>>> checkstyle currently reports a huge amount of errors. We currently
> >> have
> >>>>> an
> >>>>>> error count setup in the configuration to prevent the build from
> >> failing
> >>>>>> because of the present errors.
> >>>>>>
> >>>>>> Some thoughts/questions to the community:
> >>>>>>
> >>>>>> * should we take an approach to fine-tune the  configuration so that
> >> it
> >>>>>> better fits the project style?
> >>>>>>
> >>>>>> As an example, constants are currently not allowed to be named
> >> "module",
> >>>>>> "resource" etc. which is a common pattern in our code. Changing from
> >>>>>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to
> >> ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
> >>>>>> would allow the common naming.
> >>>>>>
> >>>>>> The opposite approach would be to rename those to fit the default
> >>>>> settings.
> >>>>>> * should we start an initiative to remove the valid errors like we
> did
> >>>>> with
> >>>>>> the FindBugs initiative some time ago?
> >>>>>>
> >>>>>>
> >>>>>> Thanks for your feedback,
> >>>>>>
> >>>>>> Michael Brohl
> >>>>>>
> >>>>>> ecomify GmbH - www.ecomify.de
> >>>>>>
> >>>>>>
> >>>>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
Your copy&paste comments from yetus have a very bad signal/noise ratio [1].

Let's see for https://issues.apache.org/jira/browse/OFBIZ-10001:

===

+1 overall


  ____                              _
/ ___| _   _  ___ ___ ___  ___ ___| |
\___ \| | | |/ __/ __/ _ \/ __/ __| |
  ___) | |_| | (_| (_|  __/\__ \__ \_|
|____/ \__,_|\___\___\___||___/___(_)
                                     




| Vote |      Subsystem |Runtime    | Comment
============================================================================
|   0  |        reexec  |   0m 47s   | Docker mode activated.
+---------------------------------------------------------------------------
|      |                |            | Prechecks
+---------------------------------------------------------------------------
|  +1  |       dupname  |   0m  0s   | Nocase  conflicting files found.
|  +1  |       @author  |   0m  1s   | The patch does not contain any @author
|      |                |            | tags.
+---------------------------------------------------------------------------
|      |                |            | trunk Compile Tests
+---------------------------------------------------------------------------
+---------------------------------------------------------------------------
|      |                |            | Patch Compile Tests
+---------------------------------------------------------------------------
|  +1  |    whitespace  |   0m  0s   | The patch has no whitespace issues.
|  +1  |           xml  |   0m  3s   | The patch has no ill-formed XML file.
+---------------------------------------------------------------------------
|      |                |            | Other Tests
+---------------------------------------------------------------------------
|   0  |    asflicense  |   1m  2s   | ASF License check generated no output?
|      |                |  27m 43s   |




|| Subsystem || Report/Notes ||
============================================================================
| Docker | Client=18.09.2 Server=18.09.2 base: /Users/nl04748/dev/test/yetus/ofbiz/OFBIZ-10001/Dockerfile |
| JIRA Issue | OFBIZ-10001 |
| JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12911530/OFBIZ-10001_org.apache.ofbiz.product.category.CategoryContentServices_convert.patch 
|

===

22/44 lines of the output have no value at all while the other lines IMO
have little value.

- What's the use of the information that there is no ill-formed xml when
the patch does not contain any xml at all?

- What does it provide to the reader that the ASF license check does not
generated output while the patch contains the ASF license header?

- it was also mentioned before that the current patch will be reworked
so the information was outdated while generated


If you provide such information in this format for every patch attached
in a Jira, it will become literally unreadable.

Can you explain where you see the value for the mentioned audience? I
doubt that they even read it.

By the way, the same bad signal/noise ratio applies to your signature.

Michael


[1] https://en.wikipedia.org/wiki/Signal-to-noise_ratio



Am 19.02.20 um 16:35 schrieb Pierre Smits:

> What you find cluttering, I regards as helpful to:
>
>     - fellow contributors
>     - our (potential) adopters
>     - anyone else working on an OFBiz implementation
>
>
> Met vriendelijke groet,
>
> Pierre Smits
> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
> <https://ofbiz.apache.org/>, since 2008
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>
>
> On Wed, Feb 19, 2020 at 3:54 PM Michael Brohl <[hidden email]>
> wrote:
>
>> No, I did not say or mean anything like that. Not sure where you get
>> that from.
>>
>> Please read again, thanks,
>>
>> Michael
>>
>> Am 19.02.20 um 15:51 schrieb Pierre Smits:
>>> Michael,
>>>
>>> You mean like:
>>>
>>>      - *stop* posting comments to tickets to help fellow contributors and
>> the
>>>      adopters of our product looking to these contributions to help them
>> decide
>>>      whether or not to invest in an adoption/implementation of (or
>> looking for
>>>      guidance on issues they experience with their implementation of) the
>>>      product?
>>>      - *stop* posting to mail threads to provide insight and such,?
>>>      - *stop* providing reactions to comments posted to pages in our wiki?
>>>      - *stop* taking the initiative?
>>>
>>> Met vriendelijke groet,
>>>
>>> Pierre Smits
>>> *Proud* *contributor* (but unfortunately without privileges)* of* Apache
>>> OFBiz <https://ofbiz.apache.org/>, since 2008
>>>
>>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
>>> *Apache Directory <https://directory.apache.org>, PMC Member*
>>> Apache Incubator <https://incubator.apache.org>, committer
>>> Apache Steve <https://steve.apache.org>, committer
>>>
>>>
>>> On Wed, Feb 19, 2020 at 3:05 PM Michael Brohl <[hidden email]>
>>> wrote:
>>>
>>>> +1 Gil
>>>>
>>>> Pierre, can you please stop cluttering the issues with the copy/pasted
>>>> yetus artifacts from your local environment?
>>>>
>>>> At least until we decided to use it and have a setup on the official
>>>> infrastructure.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 14.02.20 um 16:34 schrieb Gil Portenseigne:
>>>>> Hello Pierre,
>>>>>
>>>>> It is interesting but i guess that will be the next step.
>>>>>
>>>>> Thanks
>>>>>
>>>>> On Fri, Feb 14, 2020 at 09:04:49AM +0100, Pierre Smits wrote:
>>>>>> It seems many projects (including those Apache projects in the Hadoop
>>>>>> sphere) use the works of the Apache Yetus (see [1]) project to do
>>>>>> pre-commit checks (including IIUC checkstyle, in an automate way) on
>>>> patch
>>>>>> files attached to JIRA tickets and Pull Requests in Github against the
>>>>>> latest commit in the branch. This may be helpful to lessen the burden
>> on
>>>>>> the contributor. When configured correctly, the test results appear in
>>>> the
>>>>>> ticket. Apache HBASE is such a project using Yetus, See as an example
>>>> [2]
>>>>>> where test results are included through comments of 'Hadoop QA'.
>>>>>>
>>>>>> Maybe we should consider this for our project?
>>>>>>
>>>>>>
>>>>>> [1] https://yetus.apache.org
>>>>>> [2]
>>>>>>
>> https://issues.apache.org/jira/browse/HBASE-14498?jql=project%20%3D%20HBASE%20AND%20status%20%3D%20%22Patch%20Available%22%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC
>>>>>> Best regards,
>>>>>>
>>>>>> Pierre Smits
>>>>>> *Proud* *contributor* (but without privileges)* of* Apache OFBiz
>>>>>> <https://ofbiz.apache.org/>, since 2008
>>>>>>
>>>>>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
>>>>>> *Apache Directory <https://directory.apache.org>, PMC Member*
>>>>>> Apache Incubator <https://incubator.apache.org>, committer
>>>>>> Apache Steve <https://steve.apache.org>, committer
>>>>>>
>>>>>>
>>>>>> On Thu, Feb 13, 2020 at 9:13 PM Gil Portenseigne <
>>>>>> [hidden email]> wrote:
>>>>>>
>>>>>>> Hello Michael,
>>>>>>>
>>>>>>> Adapting checkstyle configuration is less impacting to our codebase
>> but
>>>>>>> make us stay different from the java standard.
>>>>>>> That is the easier path, that will not affect code history.
>>>>>>>
>>>>>>> But about getting nearer from the java standard is IMO a nice to
>> have,
>>>> to
>>>>>>> make pure java developer feel better discovering OFBiz.
>>>>>>> I'm kinda prefer the "opposite approach", but we need to discuss if
>>>> this
>>>>>>> improvement is worth the history lost.
>>>>>>>
>>>>>>> In the example you chose, i see no issue capitalizing module,
>>>>>>> resource and other. Updating the rule offer the ability to write a
>>>>>>> constant like : MoDuLe  :-).
>>>>>>>
>>>>>>> Lots of errors are about line that are 120+ length, missing or
>> uneeded
>>>>>>> space etc.
>>>>>>>
>>>>>>> So that will lead to loads of unrisky modifications. With the usage
>> of
>>>>>>> IDE with checkstyle plugin this can be done nicely.
>>>>>>> I check that over the near thousand of java file concerned more than
>>>> 80%
>>>>>>> have less than 50 issue... So we can focus our effort onto the big
>>>> files
>>>>>>> using a branch to test the idea and ease the issue.
>>>>>>>
>>>>>>> And like Daniel said, it is a good subject for a community week
>> effort.
>>>>>>> I'd really like to be able to efficiently use this feature, in one
>> way
>>>>>>> or the other, so thank you for starting this discussion.
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Gil
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Feb 13, 2020 at 05:44:40PM +0100, Michael Brohl wrote:
>>>>>>>> Hi *,
>>>>>>>>
>>>>>>>> checkstyle currently reports a huge amount of errors. We currently
>>>> have
>>>>>>> an
>>>>>>>> error count setup in the configuration to prevent the build from
>>>> failing
>>>>>>>> because of the present errors.
>>>>>>>>
>>>>>>>> Some thoughts/questions to the community:
>>>>>>>>
>>>>>>>> * should we take an approach to fine-tune the  configuration so that
>>>> it
>>>>>>>> better fits the project style?
>>>>>>>>
>>>>>>>> As an example, constants are currently not allowed to be named
>>>> "module",
>>>>>>>> "resource" etc. which is a common pattern in our code. Changing from
>>>>>>>> ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ to
>>>> ^[a-zA-Z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$
>>>>>>>> would allow the common naming.
>>>>>>>>
>>>>>>>> The opposite approach would be to rename those to fit the default
>>>>>>> settings.
>>>>>>>> * should we start an initiative to remove the valid errors like we
>> did
>>>>>>> with
>>>>>>>> the FindBugs initiative some time ago?
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for your feedback,
>>>>>>>>
>>>>>>>> Michael Brohl
>>>>>>>>
>>>>>>>> ecomify GmbH - www.ecomify.de
>>>>>>>>
>>>>>>>>
>>


smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
As always: with good intentions often comes bad execution...

You shouldn't have had to post that from the ticket as people can follow
the reference. Unless you want to drive home the point you're trying to
make about the S/N ratio.

As for 3rd party tools used, whatever/wherever/whenever, I am confident
that all have that S/N ratio. For some it may be perceived higher than for
others I suggest that, if you want to reduce that, you go knock on their
door and 'suggest improvements' there.. I am equally confident that the
folks at Apache Yetus will welcome your critique.

Met vriendelijke groet,

Pierre Smits
*Proud* *contributor* (but unfortunately without privileges)* of* Apache
OFBiz <https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Michael Brohl-3
Yetus has nothing to do with your copy & paste work so responsibility is
on your side.

You did not answer any of my questions to understand the values you see
in the *concrete* example.

Michael


Am 19.02.20 um 18:36 schrieb Pierre Smits:

> As always: with good intentions often comes bad execution...
>
> You shouldn't have had to post that from the ticket as people can follow
> the reference. Unless you want to drive home the point you're trying to
> make about the S/N ratio.
>
> As for 3rd party tools used, whatever/wherever/whenever, I am confident
> that all have that S/N ratio. For some it may be perceived higher than for
> others I suggest that, if you want to reduce that, you go knock on their
> door and 'suggest improvements' there.. I am equally confident that the
> folks at Apache Yetus will welcome your critique.
>
> Met vriendelijke groet,
>
> Pierre Smits
> *Proud* *contributor* (but unfortunately without privileges)* of* Apache
> OFBiz <https://ofbiz.apache.org/>, since 2008
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>


smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: checkstyle improvements

Pierre Smits-3
You are correct about me not answering your questions here, as it would add
to an already deteriorating S/N ratio regarding the subject of this thread.

Met vriendelijke groet,

Pierre Smits
*Proud* *contributor* (but unfortunately without privileges)* of* Apache
OFBiz <https://ofbiz.apache.org/>, since 2008

*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer


On Wed, Feb 19, 2020 at 6:47 PM Michael Brohl <[hidden email]>
wrote:

> Yetus has nothing to do with your copy & paste work so responsibility is
> on your side.
>
> You did not answer any of my questions to understand the values you see
> in the *concrete* example.
>
> Michael
>
>
> Am 19.02.20 um 18:36 schrieb Pierre Smits:
> > As always: with good intentions often comes bad execution...
> >
> > You shouldn't have had to post that from the ticket as people can follow
> > the reference. Unless you want to drive home the point you're trying to
> > make about the S/N ratio.
> >
> > As for 3rd party tools used, whatever/wherever/whenever, I am confident
> > that all have that S/N ratio. For some it may be perceived higher than
> for
> > others I suggest that, if you want to reduce that, you go knock on their
> > door and 'suggest improvements' there.. I am equally confident that the
> > folks at Apache Yetus will welcome your critique.
> >
> > Met vriendelijke groet,
> >
> > Pierre Smits
> > *Proud* *contributor* (but unfortunately without privileges)* of* Apache
> > OFBiz <https://ofbiz.apache.org/>, since 2008
> >
> > *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> > *Apache Directory <https://directory.apache.org>, PMC Member*
> > Apache Incubator <https://incubator.apache.org>, committer
> > Apache Steve <https://steve.apache.org>, committer
> >
>
>