Re: Apache Rat plugin, license for new classes

2018-10-14 Thread Allon Mureinik 
Can you link to that PR so we have something concrete to look at?

On Sun, Oct 14, 2018 at 7:49 PM Aleksander Ściborek <
aleksanderscibo...@gmail.com> wrote:

> Hi,
> During my first attempt to create pull request with 2 new classes (a single
> utils class and a test class) I have encountered problem with apache rat
> plugin:
> "Failed to execute goal org.apache.rat:apache-rat-plugin:0.12:check
> (default-cli) on project commons-lang3: Too many files with unapproved
> license"
> What should I do?
> Aleksander
>

Re: [LANG] travis build failures

2018-02-27 Thread Allon Mureinik 
An alternative approach could be to remove the randomness from the test by
using a predefined random seed and test the overloaded variants that accept
a second java.util.Random argument. This will superficially reduce the
test's coverage, but it does have a reliability advantage, IMHO (as seen in
Otto's original email - these tests do fail occasionally).

I posted https://github.com/apache/commons-lang/pull/317 to show this
approach

On Wed, Feb 28, 2018 at 12:00 AM, Gary Gregory <garydgreg...@gmail.com>
wrote:

> Of course... but how would test then? Shuffle N times and accept a % of
> non-shuffles?
>
> Gary
>
> On Tue, Feb 27, 2018, 13:18 Allon Mureinik <murei...@gmail.com> wrote:
>
> > There will still be a chance, however infinitesimal, of a failure. :-)
> >
> >
> > On Tue, Feb 27, 2018 at 9:02 PM, Gary Gregory <garydgreg...@gmail.com>
> > wrote:
> >
> > > Why not make the array 1000 items long?
> > >
> > > Gary
> > >
> > > On Tue, Feb 27, 2018 at 10:31 AM, Allon Mureinik <murei...@gmail.com>
> > > wrote:
> > >
> > > > All the ArrayUtilsTest#testShuffleXYZ tests take an array, shuffle
> it,
> > > and
> > > > assert that the result isn't equal to the original array.
> > > > This is usually true, but there's a small chance that the shuffled
> > array
> > > > will be equal to the original array, and thus the test will fail.
> This
> > > > chance is higher for the testShuffleBoolean case where the array
> > contains
> > > > ten elements, but only two distinct values (true and false).
> > > >
> > > > I've sent a PR to remove these problematic assertions, let's see what
> > the
> > > > maintainers think of it:
> > > > https://github.com/apache/commons-lang/pull/316
> > > >
> > > >
> > > > On Tue, Feb 27, 2018 at 6:59 PM, Otto Fowler <
> ottobackwa...@gmail.com>
> > > > wrote:
> > > >
> > > > > Note, this does pass in my personal travis:
> > > > > https://travis-ci.org/ottobackwards/commons-lang/builds/346806991
> > > > >
> > > > >
> > > > > On February 27, 2018 at 11:58:24, Otto Fowler (
> > ottobackwa...@gmail.com
> > > )
> > > > > wrote:
> > > > >
> > > > > My PR is currently failing for java 9 on this test.  Anyone have
> any
> > > idea
> > > > > why?
> > > > >
> > > > > [INFO] Running org.apache.commons.lang3.ArrayUtilsTest
> > > > > [ERROR] Tests run: 307, Failures: 1, Errors: 0, Skipped: 0, Time
> > > elapsed:
> > > > > 0.114 s <<< FAILURE! - in org.apache.commons.lang3.ArrayUtilsTest
> > > > > [ERROR] testShuffleBoolean(org.apache.
> commons.lang3.ArrayUtilsTest)
> > > Time
> > > > > elapsed: 0.008 s <<< FAILURE!
> > > > > java.lang.AssertionError
> > > > > at
> > > > > org.apache.commons.lang3.ArrayUtilsTest.testShuffleBoolean(
> > > > > ArrayUtilsTest.java:5023)
> > > > >
> > > >
> > >
> >
>

Re: [LANG] travis build failures

2018-02-27 Thread Allon Mureinik 
There will still be a chance, however infinitesimal, of a failure. :-)


On Tue, Feb 27, 2018 at 9:02 PM, Gary Gregory <garydgreg...@gmail.com>
wrote:

> Why not make the array 1000 items long?
>
> Gary
>
> On Tue, Feb 27, 2018 at 10:31 AM, Allon Mureinik <murei...@gmail.com>
> wrote:
>
> > All the ArrayUtilsTest#testShuffleXYZ tests take an array, shuffle it,
> and
> > assert that the result isn't equal to the original array.
> > This is usually true, but there's a small chance that the shuffled array
> > will be equal to the original array, and thus the test will fail. This
> > chance is higher for the testShuffleBoolean case where the array contains
> > ten elements, but only two distinct values (true and false).
> >
> > I've sent a PR to remove these problematic assertions, let's see what the
> > maintainers think of it:
> > https://github.com/apache/commons-lang/pull/316
> >
> >
> > On Tue, Feb 27, 2018 at 6:59 PM, Otto Fowler <ottobackwa...@gmail.com>
> > wrote:
> >
> > > Note, this does pass in my personal travis:
> > > https://travis-ci.org/ottobackwards/commons-lang/builds/346806991
> > >
> > >
> > > On February 27, 2018 at 11:58:24, Otto Fowler (ottobackwa...@gmail.com
> )
> > > wrote:
> > >
> > > My PR is currently failing for java 9 on this test.  Anyone have any
> idea
> > > why?
> > >
> > > [INFO] Running org.apache.commons.lang3.ArrayUtilsTest
> > > [ERROR] Tests run: 307, Failures: 1, Errors: 0, Skipped: 0, Time
> elapsed:
> > > 0.114 s <<< FAILURE! - in org.apache.commons.lang3.ArrayUtilsTest
> > > [ERROR] testShuffleBoolean(org.apache.commons.lang3.ArrayUtilsTest)
> Time
> > > elapsed: 0.008 s <<< FAILURE!
> > > java.lang.AssertionError
> > > at
> > > org.apache.commons.lang3.ArrayUtilsTest.testShuffleBoolean(
> > > ArrayUtilsTest.java:5023)
> > >
> >
>

Re: [LANG] travis build failures

2018-02-27 Thread Allon Mureinik 
All the ArrayUtilsTest#testShuffleXYZ tests take an array, shuffle it, and
assert that the result isn't equal to the original array.
This is usually true, but there's a small chance that the shuffled array
will be equal to the original array, and thus the test will fail. This
chance is higher for the testShuffleBoolean case where the array contains
ten elements, but only two distinct values (true and false).

I've sent a PR to remove these problematic assertions, let's see what the
maintainers think of it:
https://github.com/apache/commons-lang/pull/316


On Tue, Feb 27, 2018 at 6:59 PM, Otto Fowler 
wrote:

> Note, this does pass in my personal travis:
> https://travis-ci.org/ottobackwards/commons-lang/builds/346806991
>
>
> On February 27, 2018 at 11:58:24, Otto Fowler (ottobackwa...@gmail.com)
> wrote:
>
> My PR is currently failing for java 9 on this test.  Anyone have any idea
> why?
>
> [INFO] Running org.apache.commons.lang3.ArrayUtilsTest
> [ERROR] Tests run: 307, Failures: 1, Errors: 0, Skipped: 0, Time elapsed:
> 0.114 s <<< FAILURE! - in org.apache.commons.lang3.ArrayUtilsTest
> [ERROR] testShuffleBoolean(org.apache.commons.lang3.ArrayUtilsTest) Time
> elapsed: 0.008 s <<< FAILURE!
> java.lang.AssertionError
> at
> org.apache.commons.lang3.ArrayUtilsTest.testShuffleBoolean(
> ArrayUtilsTest.java:5023)
>

Re: [All][RNG] FindBugs discontinued

2018-02-16 Thread Allon Mureinik 
darn, missed one :-)
Thanks Giles!

On Sat, Feb 17, 2018 at 5:12 AM, Gilles <gil...@harfang.homelinux.org>
wrote:

> On Fri, 16 Feb 2018 21:12:39 +0200, Allon Mureinik wrote:
>
>> The migration from FindBugs to SpotBugs seems to be trivial once we decide
>> to go down that route:
>> https://github.com/apache/commons-rng/pull/7
>>
>
> Merged.
> Thanks for the patch, but it didn't work without an
> additional change (see log). :-}
>
> [Note that there are test case failures in the CI for this PR that are
>> unrelated to the findbugs-spotbugs migration - they also seem to reproduce
>> on master]
>>
>
> Those failures occur intermittently, due to the "random"
> nature of the functionality being tested.
>
> Regards,
> Gilles
>
>
>
>> On Fri, Feb 16, 2018 at 8:05 PM, Jacques Le Roux <
>> jacques.le.r...@les7arts.com> wrote:
>>
>> OK, so I'll wait an official announce from http://findbugs.sourceforge.ne
>>> t/
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 16/02/2018 à 17:25, Gilles a écrit :
>>>
>>> On Fri, 16 Feb 2018 15:02:32 +0100, Jacques Le Roux wrote:
>>>>
>>>> Hi Jochen,
>>>>>
>>>>> Actually Gilles suggested to use spotbugs
>>>>>
>>>>>
>>>> Yes.
>>>> The SpotBugs web site mentions that they are the successor.
>>>> And the FindBugs web site is offline/disabled.
>>>> "Commons" projects use the FindBugs maven plugin and, as
>>>> I've shown, that is going to be a problem with Java 9.
>>>>
>>>> Gilles
>>>>
>>>>
>>>> Jacques
>>>>>
>>>>>
>>>>> Le 16/02/2018 à 10:39, Jochen Wiedmann a écrit :
>>>>>
>>>>> Hi, Gilles,
>>>>>>
>>>>>> are you aware of https://spotbugs.github.io/
>>>>>>
>>>>>> Apart from the name change: A lot of commits in January. Doesn't look
>>>>>> dead to me.
>>>>>>
>>>>>> Jochen
>>>>>>
>>>>>>
>>>>>> On Fri, Feb 16, 2018 at 9:04 AM, Jacques Le Roux
>>>>>> <jacques.le.r...@les7arts.com> wrote:
>>>>>>
>>>>>> Le 14/02/2018 à 19:02, Gilles a écrit :
>>>>>>>
>>>>>>> FindBugs chokes on Java 9 "module-info" files[1] and the project has
>>>>>>>> been shut down
>>>>>>>>
>>>>>>>> Hi Gilles,
>>>>>>>
>>>>>>> Do you have a reference where it's said that FindBugs has been shut
>>>>>>> down?
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [All][RNG] FindBugs discontinued

2018-02-16 Thread Allon Mureinik 
The migration from FindBugs to SpotBugs seems to be trivial once we decide
to go down that route:
https://github.com/apache/commons-rng/pull/7

[Note that there are test case failures in the CI for this PR that are
unrelated to the findbugs-spotbugs migration - they also seem to reproduce
on master]

On Fri, Feb 16, 2018 at 8:05 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> OK, so I'll wait an official announce from http://findbugs.sourceforge.ne
> t/
>
> Thanks
>
> Jacques
>
>
>
> Le 16/02/2018 à 17:25, Gilles a écrit :
>
>> On Fri, 16 Feb 2018 15:02:32 +0100, Jacques Le Roux wrote:
>>
>>> Hi Jochen,
>>>
>>> Actually Gilles suggested to use spotbugs
>>>
>>
>> Yes.
>> The SpotBugs web site mentions that they are the successor.
>> And the FindBugs web site is offline/disabled.
>> "Commons" projects use the FindBugs maven plugin and, as
>> I've shown, that is going to be a problem with Java 9.
>>
>> Gilles
>>
>>
>>> Jacques
>>>
>>>
>>> Le 16/02/2018 à 10:39, Jochen Wiedmann a écrit :
>>>
 Hi, Gilles,

 are you aware of https://spotbugs.github.io/

 Apart from the name change: A lot of commits in January. Doesn't look
 dead to me.

 Jochen


 On Fri, Feb 16, 2018 at 9:04 AM, Jacques Le Roux
  wrote:

> Le 14/02/2018 à 19:02, Gilles a écrit :
>
>> FindBugs chokes on Java 9 "module-info" files[1] and the project has
>> been shut down
>>
> Hi Gilles,
>
> Do you have a reference where it's said that FindBugs has been shut
> down?
>
> Thanks
>
> Jacques
>

>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> For additional commands, e-mail: dev-h...@commons.apache.org
>>
>>
>>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [lang] [LANG-1360] Add methods to ObjectUtils to get various forms of class names in a null-safe manner

2017-10-21 Thread Allon Mureinik 
On Sat, Oct 21, 2017 at 6:36 PM, Benedikt Ritter <brit...@apache.org> wrote:

>
>
> > Am 21.10.2017 um 13:54 schrieb Allon Mureinik <murei...@gmail.com>:
> >
> > Hi guys,
> >
> > This patch break the CI on GitHub (the checkstyle check).
> >
> > It's pretty straight forward to fix [1], and if someone could merge it to
> > unblock the CI, that would be great.
>
> Done.
>
Many thanks!


>
> >
> >
> > TIA,
> > Allon
> >
> > [1] https://github.com/apache/commons-lang/pull/302
> >
> > On Sat, Oct 21, 2017 at 11:27 AM, Benedikt Ritter <brit...@apache.org>
> > wrote:
> >
> >> Hello Gary,
> >>
> >>> Am 20.10.2017 um 21:19 schrieb ggreg...@apache.org:
> >>>
> >>> Repository: commons-lang
> >>> Updated Branches:
> >>> refs/heads/master 88654b79c -> 6ea2fc8d3
> >>>
> >>>
> >>> [LANG-1360] Add methods to ObjectUtils to get various forms of class
> >>> names in a null-safe manner
> >>
> >> I think this belongs to ClassUtils.
> >>
> >> Regards,
> >> Benedikt
> >>
> >>>
> >>> Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
> >>> Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/
> >> 6ea2fc8d
> >>> Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/
> 6ea2fc8d
> >>> Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/
> 6ea2fc8d
> >>>
> >>> Branch: refs/heads/master
> >>> Commit: 6ea2fc8d38e035bafaa92c7d3b007be38c2e9000
> >>> Parents: 88654b7
> >>> Author: Gary Gregory <ggreg...@apache.org>
> >>> Authored: Fri Oct 20 13:19:56 2017 -0600
> >>> Committer: Gary Gregory <ggreg...@apache.org>
> >>> Committed: Fri Oct 20 13:19:56 2017 -0600
> >>>
> >>> --
> >>> src/changes/changes.xml |  1 +
> >>> .../org/apache/commons/lang3/ObjectUtils.java   | 32
> +++
> >>> .../apache/commons/lang3/ObjectUtilsTest.java   | 33
> >> 
> >>> 3 files changed, 66 insertions(+)
> >>> --
> >>>
> >>>
> >>> http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> >> 6ea2fc8d/src/changes/changes.xml
> >>> --
> >>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> >>> index cf4296e..66dd510 100644
> >>> --- a/src/changes/changes.xml
> >>> +++ b/src/changes/changes.xml
> >>> @@ -53,6 +53,7 @@ The  type attribute can be
> >> add,update,fix,remove.
> >>>ConstructorUtils.invokeConstructor(Class, Object...)
> >> regression
> >>>EqualsBuilder#isRegistered: swappedPair construction
> bug
> >>> >> due-to="BruceKuiLiu">org.apache.commons.lang3.time.FastDateParser
> should
> >> use toUpperCase(Locale)
> >>> +Add methods to ObjectUtils to get various forms of class names
> in
> >> a null-safe manner
> >>>  
> >>>
> >>>  
> >>>
> >>> http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> >> 6ea2fc8d/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> >>> --
> >>> diff --git a/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> >> b/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> >>> index 1ec0956..16a6b93 100644
> >>> --- a/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> >>> +++ b/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> >>> @@ -1033,4 +1033,36 @@ public class ObjectUtils {
> >>>return v;
> >>>}
> >>>
> >>> +/**
> >>> + * Gets the class name of the given object.
> >>> + *
> >>> + * @param object the object to query, may be null
> >>> + * @return the given object's class name or null if the object is
> >> null
> >>> + * @since 3.7
> >>> + */
> >>> +public static String getClassName(final Object object) {
> >>> +return object

Re: [lang] [LANG-1360] Add methods to ObjectUtils to get various forms of class names in a null-safe manner

2017-10-21 Thread Allon Mureinik 
Hi guys,

This patch break the CI on GitHub (the checkstyle check).

It's pretty straight forward to fix [1], and if someone could merge it to
unblock the CI, that would be great.


TIA,
Allon

[1] https://github.com/apache/commons-lang/pull/302

On Sat, Oct 21, 2017 at 11:27 AM, Benedikt Ritter 
wrote:

> Hello Gary,
>
> > Am 20.10.2017 um 21:19 schrieb ggreg...@apache.org:
> >
> > Repository: commons-lang
> > Updated Branches:
> >  refs/heads/master 88654b79c -> 6ea2fc8d3
> >
> >
> > [LANG-1360] Add methods to ObjectUtils to get various forms of class
> > names in a null-safe manner
>
> I think this belongs to ClassUtils.
>
> Regards,
> Benedikt
>
> >
> > Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/
> 6ea2fc8d
> > Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/6ea2fc8d
> > Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/6ea2fc8d
> >
> > Branch: refs/heads/master
> > Commit: 6ea2fc8d38e035bafaa92c7d3b007be38c2e9000
> > Parents: 88654b7
> > Author: Gary Gregory 
> > Authored: Fri Oct 20 13:19:56 2017 -0600
> > Committer: Gary Gregory 
> > Committed: Fri Oct 20 13:19:56 2017 -0600
> >
> > --
> > src/changes/changes.xml |  1 +
> > .../org/apache/commons/lang3/ObjectUtils.java   | 32 +++
> > .../apache/commons/lang3/ObjectUtilsTest.java   | 33
> 
> > 3 files changed, 66 insertions(+)
> > --
> >
> >
> > http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> 6ea2fc8d/src/changes/changes.xml
> > --
> > diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> > index cf4296e..66dd510 100644
> > --- a/src/changes/changes.xml
> > +++ b/src/changes/changes.xml
> > @@ -53,6 +53,7 @@ The  type attribute can be
> add,update,fix,remove.
> > ConstructorUtils.invokeConstructor(Class, Object...)
> regression
> > EqualsBuilder#isRegistered: swappedPair construction bug
> >  due-to="BruceKuiLiu">org.apache.commons.lang3.time.FastDateParser should
> use toUpperCase(Locale)
> > +Add methods to ObjectUtils to get various forms of class names in
> a null-safe manner
> >   
> >
> >   
> >
> > http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> 6ea2fc8d/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> > --
> > diff --git a/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> b/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> > index 1ec0956..16a6b93 100644
> > --- a/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> > +++ b/src/main/java/org/apache/commons/lang3/ObjectUtils.java
> > @@ -1033,4 +1033,36 @@ public class ObjectUtils {
> > return v;
> > }
> >
> > +/**
> > + * Gets the class name of the given object.
> > + *
> > + * @param object the object to query, may be null
> > + * @return the given object's class name or null if the object is
> null
> > + * @since 3.7
> > + */
> > +public static String getClassName(final Object object) {
> > +return object == null ? null : object.getClass().getName();
> > +}
> > +
> > +/**
> > + * Gets the class simple name of the given object.
> > + *
> > + * @param object the object to query, may be null
> > + * @return the given object's class simple name or null if the
> object is null
> > + * @since 3.7
> > + */
> > +public static String getClassSimpleName(final Object object) {
> > +return object == null ? null : object.getClass().
> getSimpleName();
> > +}
> > +
> > +/**
> > + * Gets the class canonical name of the given object.
> > + *
> > + * @param object the object to query, may be null
> > + * @return the given object's class canonical name or null if the
> object is null
> > + * @since 3.7
> > + */
> > +public static String getClassCanonicalName(final Object object) {
> > +return object == null ? null : object.getClass().
> getCanonicalName();
> > +}
> > }
> >
> > http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> 6ea2fc8d/src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
> > --
> > diff --git a/src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
> b/src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
> > index 3da8443..2bf036c 100644
> > --- a/src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
> > +++ b/src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
> > @@ -38,6 +38,7 @@ import java.util.List;
> > import

Re: [ALL] Automated requirements (e.g. CheckStyle)? (Was: [MATH] Enforce run [...])

2017-08-07 Thread Allon Mureinik 
On Mon, Aug 7, 2017 at 2:36 PM, Gilles <gil...@harfang.homelinux.org> wrote:

> On Mon, 7 Aug 2017 14:08:45 +0300, Allon Mureinik wrote:
>
>> On Mon, Aug 7, 2017 at 12:16 PM, Gilles <gil...@harfang.homelinux.org>
>> wrote:
>>
>> Hello.
>>>
>>> On Fri, 4 Aug 2017 21:17:43 +0300, Allon Mureinik wrote:
>>>
>>> We had a similar discussion about Configuration.
>>>>
>>>> Personally, I'm all for enforcing checkstyle during the validate phase,
>>>> but
>>>> we couldn't reach a consensus about it there:
>>>> http://www.mail-archive.com/dev@commons.apache.org/msg58573.html
>>>>
>>>> On Fri, Aug 4, 2017 at 7:16 PM, Karl-Philipp Richter <
>>>> krich...@posteo.de>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>>> While working on a [small
>>>>> contribution](https://issues.apache.org/jira/browse/MATH-1426) I
>>>>> noticed
>>>>> that there's a checkstyle setup which is run in a reporting phase of
>>>>> Maven which might be skipped by most developers and isn't used on
>>>>> Travis
>>>>> CI.
>>>>>
>>>>>
>>>> Well, running
>>>   $ mvn site
>>> and checking the reports, is one of the (unwritten?) rule a contributor
>>> to Commons will be told about. ;-)
>>>
>>>
>> I think unwritten rules are worth as much as the paper they're written on.
>> Running "mvn site" is a great practice, but if we expect people to do so,
>> it should be stated very clearly in the contributor's guide.
>>
>
> Perhaps this is the document to update:
>   https://commons.apache.org/patches.html
> ?
>
>
> I suggest to move this phase to the validate phase of Maven which
>>>
>>>> runs before the compile and test phase and in case of failure forbids
>>>>> the invoker to build the project successfully.
>>>>>
>>>>>
>>>> Forcing style even before the compiler has the chance to warn about
>>> invalid code looks a bit strong.
>>>
>>> Therefore most
>>>
>>>> contributions will like they're intended too without the need of extra
>>>>> communication.
>>>>>
>>>>>
>>>> If the above rule can be improved over, fine, but running CheckStyle
>>> takes time (e.g. wrt to the compilation of a fix being worked on); so
>>> it should be "mandatory" only before submitting a contribution.
>>>
>>> Perhaps, we should have a maven profile "-P check-requirements"
>>> which contributors _must_ run before providing a PR or attaching a
>>> patch to JIRA.
>>> That profile would thus contain your suggestion.
>>>
>>>
>>> The downside is that new (and eventually old) devs might be annoyed at
>>>>> some point, especially if they frequently work on different projects
>>>>> with different styles.
>>>>>
>>>>>
>>>> Indeed, hence my suggestion to not change the usual workflow but to
>>> advertize that contribution will be taken into account only if they
>>> pass the requirements.
>>>
>>>
>> I think there's two needs we should answer here.
>>
>> First, maintainers shouldn't have to run any additional step in order to
>> decide whether a patch is worthy. They should look at the code and commit
>> message(s), adn see if they have their merrit. Any technical requirement
>> (compilation, code style, test coverage, etc) can and should be handled by
>> automatic tools (namely: CI, or to be more specific, Travis CI we're using
>> on GitHub). Unless it takes hours upon hours to run (which it shouldn't),
>> the  fact that it takes time is inconsequential. You submit your patch,
>> and
>> once CI have verified that it's up to standard (usually within seveal
>> minutes), a maintainer can take a look and judge the subtance of the
>> patch.
>> This way, maintainers don't waste their time on boilerplate commenting.
>>
>
> Less work for the maintainers is good. :-)
>
> By "taking time" I meant that validating should not be enforced when
> calling "mvn compile" or "mvn test".
>
> Second, contributors need to be made aware of the expectations. I.e., a
>> contributor should know that if he or she runs command line X (regardless
>> of whether it's "mvn install -Pcheckstyle", "mvn site" or even &qu

Re: [ALL] Automated requirements (e.g. CheckStyle)? (Was: [MATH] Enforce run [...])

2017-08-07 Thread Allon Mureinik 
On Mon, Aug 7, 2017 at 12:16 PM, Gilles <gil...@harfang.homelinux.org>
wrote:

> Hello.
>
> On Fri, 4 Aug 2017 21:17:43 +0300, Allon Mureinik wrote:
>
>> We had a similar discussion about Configuration.
>>
>> Personally, I'm all for enforcing checkstyle during the validate phase,
>> but
>> we couldn't reach a consensus about it there:
>> http://www.mail-archive.com/dev@commons.apache.org/msg58573.html
>>
>> On Fri, Aug 4, 2017 at 7:16 PM, Karl-Philipp Richter <krich...@posteo.de>
>> wrote:
>>
>> Hi,
>>> While working on a [small
>>> contribution](https://issues.apache.org/jira/browse/MATH-1426) I noticed
>>> that there's a checkstyle setup which is run in a reporting phase of
>>> Maven which might be skipped by most developers and isn't used on Travis
>>> CI.
>>>
>>
> Well, running
>   $ mvn site
> and checking the reports, is one of the (unwritten?) rule a contributor
> to Commons will be told about. ;-)
>

I think unwritten rules are worth as much as the paper they're written on.
Running "mvn site" is a great practice, but if we expect people to do so,
it should be stated very clearly in the contributor's guide.


> I suggest to move this phase to the validate phase of Maven which
>>> runs before the compile and test phase and in case of failure forbids
>>> the invoker to build the project successfully.
>>>
>>
> Forcing style even before the compiler has the chance to warn about
> invalid code looks a bit strong.
>
> Therefore most
>>> contributions will like they're intended too without the need of extra
>>> communication.
>>>
>>
> If the above rule can be improved over, fine, but running CheckStyle
> takes time (e.g. wrt to the compilation of a fix being worked on); so
> it should be "mandatory" only before submitting a contribution.
>
> Perhaps, we should have a maven profile "-P check-requirements"
> which contributors _must_ run before providing a PR or attaching a
> patch to JIRA.
> That profile would thus contain your suggestion.
>
>
>>> The downside is that new (and eventually old) devs might be annoyed at
>>> some point, especially if they frequently work on different projects
>>> with different styles.
>>>
>>
> Indeed, hence my suggestion to not change the usual workflow but to
> advertize that contribution will be taken into account only if they
> pass the requirements.
>

I think there's two needs we should answer here.

First, maintainers shouldn't have to run any additional step in order to
decide whether a patch is worthy. They should look at the code and commit
message(s), adn see if they have their merrit. Any technical requirement
(compilation, code style, test coverage, etc) can and should be handled by
automatic tools (namely: CI, or to be more specific, Travis CI we're using
on GitHub). Unless it takes hours upon hours to run (which it shouldn't),
the  fact that it takes time is inconsequential. You submit your patch, and
once CI have verified that it's up to standard (usually within seveal
minutes), a maintainer can take a look and judge the subtance of the patch.
This way, maintainers don't waste their time on boilerplate commenting.

Second, contributors need to be made aware of the expectations. I.e., a
contributor should know that if he or she runs command line X (regardless
of whether it's "mvn install -Pcheckstyle", "mvn site" or even "mvn
giles"), there are pretty good chance that the CI will also pass, assuming
there isn't some problem that only occurs on an alternative jdk/platform.


>
>
>>> I can take over the move to the validate phase which is 10 lines
>>> insertion/deletion in pom.xml, but not the definition of code style
>>> rules which are common for the project because I don't know them. Doing
>>> this change reveals about 400 issues of which > 95% are related to
>>> missing or errornous Javadoc which is worth having a look at, but might
>>> be postponed by deactivating the rule for now. Then you need to discuss
>>> code style rules, because some, like the ones in the issue linked above,
>>> aren't covered yet.
>>>
>>
> For "Commons Math", there is a custom "checkstyle.xml" (in the top
> directory of the code repository).
> When running "mvn site", CheckStyle currently reports 1 error.
>
> The numerous errors you see (but which I do not) might be in the "test"
> part of the source repository. [Historically, code style there was much
> less emphasized (and perhaps checking it is disabled by in "mvn site").]
>
> Best regards,
> Gilles
>
>
>>> -Kalle Richter
>>>
>>>
>>>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [MATH] Enforce run of checkstyle-maven-plugin in validate instead of site phase

2017-08-04 Thread Allon Mureinik 
We had a similar discussion about Configuration.

Personally, I'm all for enforcing checkstyle during the validate phase, but
we couldn't reach a consensus about it there:
http://www.mail-archive.com/dev@commons.apache.org/msg58573.html

On Fri, Aug 4, 2017 at 7:16 PM, Karl-Philipp Richter 
wrote:

> Hi,
> While working on a [small
> contribution](https://issues.apache.org/jira/browse/MATH-1426) I noticed
> that there's a checkstyle setup which is run in a reporting phase of
> Maven which might be skipped by most developers and isn't used on Travis
> CI. I suggest to move this phase to the validate phase of Maven which
> runs before the compile and test phase and in case of failure forbids
> the invoker to build the project successfully. Therefore most
> contributions will like they're intended too without the need of extra
> communication.
>
> The downside is that new (and eventually old) devs might be annoyed at
> some point, especially if they frequently work on different projects
> with different styles.
>
> I can take over the move to the validate phase which is 10 lines
> insertion/deletion in pom.xml, but not the definition of code style
> rules which are common for the project because I don't know them. Doing
> this change reveals about 400 issues of which > 95% are related to
> missing or errornous Javadoc which is worth having a look at, but might
> be postponed by deactivating the rule for now. Then you need to discuss
> code style rules, because some, like the ones in the issue linked above,
> aren't covered yet.
>
> -Kalle Richter
>
>

Re: Attaching Apache repository pushes to GitHub page

2017-08-03 Thread Allon Mureinik 
Assuming you're using the same email to contribute to commons-numbers as
you use for your GitHub profile, staring the Apache repository should do
the trick.
See also [1] for some other common pitfalls that you may have encountered

[1]
https://help.github.com/articles/why-are-my-contributions-not-showing-up-on-my-profile/

On Thu, Aug 3, 2017 at 2:38 PM, Eric Barnhill 
wrote:

> I am applying for jobs soon and want my work on commons-numbers to be
> visible on my GitHub page. However I am pushing to an Apache repository
> rather than to my own fork, so it doesn't show up on my GitHub page. Is
> there a way to link the two?
>

Re: [lang] Applying IntelliJ IDEA refactorings

2017-07-05 Thread Allon Mureinik 
I've submitted several such cleanups over the past couple of month, and for
the most part, they've been well received.

I think the key here is to improve the codebase when possible but to leave
room to deviate from IntelliJ's norms when there's a good reason to.
Perhaps annotating such places with @SuppressWarning would be the best
approach, to signal to future developers that the warning was considered,
and we explicitly decided to suppress it (possibly with a comment in the
code explaining why).


On Wed, Jul 5, 2017 at 6:42 PM, Matt Sicker  wrote:

> I like the idea myself at least, though I'm not sure if it conflicts with
> any coding styleguides established.
>
> On 4 July 2017 at 18:18, Jonathan Bluett-Duncan 
> wrote:
>
> > Hi all,
> >
> > I'm interested in going through commons-lang with IntelliJ IDEA and
> > applying small refactorings to make the code base easier to read and/or
> > more performant (and also make IntelliJ IDEA itself report less
> warnings).
> >
> > Is this something that the [lang] team would find useful?
> >
> > Examples of refactorings that I could apply include:
> >
> >- Replace manual array-to-collection copy operations with
> >`Collections.addAll()`.
> >- Replace simple `String{Buffer,Builder}` usages with direct `String`
> >concatenations or `String.format()`.
> >- Simplifying boolean expressions like `obj instanceof CharRange ==
> >false` to `!(obj instanceof CharRange)`.
> >
> >
> > Cheers,
> > Jonathan
> >
>
>
>
> --
> Matt Sicker 
>

Re: [configuration] checkstyle fails build

2017-06-23 Thread Allon Mureinik 
Instead of a per-IDE specific file, it may be useful to look into
EditorConfig [1]. A lot of IDEs support it either natively or via plugin,
and it may help having to replicate the same styling "logic" per IDE.

[1] http://editorconfig.org/

On Fri, Jun 23, 2017 at 7:35 PM, sebb <seb...@gmail.com> wrote:

> On 23 June 2017 at 17:29, Gary Gregory <garydgreg...@gmail.com> wrote:
> > On Fri, Jun 23, 2017 at 8:21 AM, Claude Warren <cla...@xenei.com> wrote:
> >
> >> How about an eclipse format configuration that will correct the error on
> >> demand.  Granted you have to run eclipse but if such a file were created
> >> (and checked in) then it would be easy for anyone running eclipse to fix
> >> it.
> >>
> >
> > I use Eclipse and would appreciate such a file. In the past we've not
> > included IDE files in the repo but making it easier would be nice.
>
> If added, these should be optional.
> i.e. don't use the same name as Eclipse uses, but copy the required
> settings to another file.
>
> See for example how Tomcat do it:
>
> http://svn.apache.org/repos/asf/tomcat/trunk/res/ide-support/eclipse/
>
> > Gary
> >
> >>
> >> Claude
> >>
> >> On Fri, Jun 23, 2017 at 4:03 PM, Oliver Heger <
> >> oliver.he...@oliver-heger.de>
> >> wrote:
> >>
> >> >
> >> >
> >> > Am 23.06.2017 um 08:58 schrieb Allon Mureinik:
> >> > > The root cause, IMHO, is having failValidation=false configured in
> the
> >> > > pom.xml. This way, when you introduce a new problem your only
> option to
> >> > > notice it is if you visually scan mvn's output. As evident by the
> >> current
> >> > > state of the build, not everyone notices these.
> >> > > A more robust approach would be to set failValidation=true, and
> >> actively
> >> > > fail the build if checkstyle's rules are violated.
> >> > >
> >> > > I've submitted a PR to fix all the existing issues and enable this
> >> > > validation. Reviews are welcome:
> >> > > https://github.com/apache/commons-configuration/pull/5
> >> > >
> >> >
> >> > Thanks for the PR, I will have a look.
> >> >
> >> > However, letting the build fail because of checkstyle error is too
> >> > restrictive IMHO. My approach is to work through the errors before
> >> > creating a new release. This has the disadvantage that errors might
> >> > accumulate; but from one release to the next one there is typically
> not
> >> > that much.
> >> >
> >> > Oliver
> >> >
> >> > >
> >> > > On Thu, Jun 22, 2017 at 11:10 PM, Gary Gregory <
> garydgreg...@gmail.com
> >> >
> >> > > wrote:
> >> > >
> >> > >> FYI, to whom can take the time to fix this.
> >> > >>
> >> > >> When I run 'mvn clean install', I get:
> >> > >>
> >> > >> [INFO] --- maven-checkstyle-plugin:2.15:check (default) @
> >> > >> commons-configuration2 ---
> >> > >> [INFO] There are 23 errors reported by Checkstyle 6.1.1 with
> >> > >> C:\vcs\svn\apache\commons\trunks-proper\configuration/
> >> > conf/checkstyle.xml
> >> > >> ruleset.
> >> > >> [ERROR]
> >> > >> src\main\java\org\apache\commons\configuration2\
> >> > >> AbstractHierarchicalConfiguration.java[976]
> >> > >> (regexp) RegexpSingleline: Line has trailing spaces.
> >> > >> [ERROR]
> >> > >> src\main\java\org\apache\commons\configuration2\
> >> > >> AbstractHierarchicalConfiguration.java[978:30]
> >> > >> (blocks) LeftCurly: '{' should be on a new line.
> >> > >> [ERROR]
> >> > >> src\main\java\org\apache\commons\configuration2\
> >> > >> AbstractYAMLBasedConfiguration.java[0]
> >> > >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> > >> [ERROR]
> >> > >> src\main\java\org\apache\commons\configuration2\builder\fluent\
> >> > >> INIBuilderParameters.java[0]
> >> > >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> > >> [ERROR]
> >> > >> src\main\java\org\apache\commons\configuration2\builder\
> >> > >> INIBuilderParamete

Re: [configuration] checkstyle fails build

2017-06-23 Thread Allon Mureinik 
On Fri, Jun 23, 2017 at 6:03 PM, Oliver Heger <oliver.he...@oliver-heger.de>
wrote:

>
>
> Am 23.06.2017 um 08:58 schrieb Allon Mureinik:
> > The root cause, IMHO, is having failValidation=false configured in the
> > pom.xml. This way, when you introduce a new problem your only option to
> > notice it is if you visually scan mvn's output. As evident by the current
> > state of the build, not everyone notices these.
> > A more robust approach would be to set failValidation=true, and actively
> > fail the build if checkstyle's rules are violated.
> >
> > I've submitted a PR to fix all the existing issues and enable this
> > validation. Reviews are welcome:
> > https://github.com/apache/commons-configuration/pull/5
> >
>
> Thanks for the PR, I will have a look.
>
> However, letting the build fail because of checkstyle error is too
> restrictive IMHO. My approach is to work through the errors before
> creating a new release. This has the disadvantage that errors might
> accumulate; but from one release to the next one there is typically not
> that much.
>
> Oliver
>

This is ultimately a matter of taste, but let me try to explain this point
of view better.
The baseline is that the project should pass checkstyle with no issues (the
first three patches in this PR will get us there).

>From that point on, the goal is not accept any patch that would break the
styling.
Think of it way - If you were reviewing a patch that didn't comply to the
project's style, you'd comment about it and ask the author to fix it before
merging.
Having checkstyle do this *as part of the CI* has the same effect, but it
takes some responsibility off the maintainers' shoulders.
First, contributers can easily see if they need to improve their patch by
running mvn install (arguably, they could do this even without enabling
checkstyle validations, but it makes it much easier to notice). Second, and
more importantly, it allows the CI to block such patches, so maintainers
can decide whether to reject them (or even fix them themselves, if they are
so inclined) so that checking checkstyle before the release becomes a
non-issue - it just always passes.



>
> >
> > On Thu, Jun 22, 2017 at 11:10 PM, Gary Gregory <garydgreg...@gmail.com>
> > wrote:
> >
> >> FYI, to whom can take the time to fix this.
> >>
> >> When I run 'mvn clean install', I get:
> >>
> >> [INFO] --- maven-checkstyle-plugin:2.15:check (default) @
> >> commons-configuration2 ---
> >> [INFO] There are 23 errors reported by Checkstyle 6.1.1 with
> >> C:\vcs\svn\apache\commons\trunks-proper\configuration/conf/
> checkstyle.xml
> >> ruleset.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\
> >> AbstractHierarchicalConfiguration.java[976]
> >> (regexp) RegexpSingleline: Line has trailing spaces.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\
> >> AbstractHierarchicalConfiguration.java[978:30]
> >> (blocks) LeftCurly: '{' should be on a new line.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\
> >> AbstractYAMLBasedConfiguration.java[0]
> >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\builder\fluent\
> >> INIBuilderParameters.java[0]
> >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\builder\
> >> INIBuilderParametersImpl.java[0]
> >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\builder\
> >> INIBuilderParametersImpl.java[42:5]
> >> (whitespace) FileTabCharacter: File contains tab characters (this is the
> >> first instance).
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\builder\
> >> INIBuilderParametersImpl.java[52:84]
> >> (blocks) LeftCurly: '{' should be on a new line.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\
> >> builder\INIBuilderProperties.java[0]
> >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\ex\
> >> ConfigurationRuntimeException.java[68]
> >> (regexp) RegexpSingleline: Line has trailing spaces.
> >> [ERROR]
> >> src\main\java\org\apache\commons\configuration2\JSONConfigur
> ation.java[0]
> >> (misc) NewlineAtEndOfFile: File does not end with a newline.
> >

Re: [configuration] checkstyle fails build

2017-06-23 Thread Allon Mureinik 
The root cause, IMHO, is having failValidation=false configured in the
pom.xml. This way, when you introduce a new problem your only option to
notice it is if you visually scan mvn's output. As evident by the current
state of the build, not everyone notices these.
A more robust approach would be to set failValidation=true, and actively
fail the build if checkstyle's rules are violated.

I've submitted a PR to fix all the existing issues and enable this
validation. Reviews are welcome:
https://github.com/apache/commons-configuration/pull/5


On Thu, Jun 22, 2017 at 11:10 PM, Gary Gregory 
wrote:

> FYI, to whom can take the time to fix this.
>
> When I run 'mvn clean install', I get:
>
> [INFO] --- maven-checkstyle-plugin:2.15:check (default) @
> commons-configuration2 ---
> [INFO] There are 23 errors reported by Checkstyle 6.1.1 with
> C:\vcs\svn\apache\commons\trunks-proper\configuration/conf/checkstyle.xml
> ruleset.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> AbstractHierarchicalConfiguration.java[976]
> (regexp) RegexpSingleline: Line has trailing spaces.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> AbstractHierarchicalConfiguration.java[978:30]
> (blocks) LeftCurly: '{' should be on a new line.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> AbstractYAMLBasedConfiguration.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\builder\fluent\
> INIBuilderParameters.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\builder\
> INIBuilderParametersImpl.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\builder\
> INIBuilderParametersImpl.java[42:5]
> (whitespace) FileTabCharacter: File contains tab characters (this is the
> first instance).
> [ERROR]
> src\main\java\org\apache\commons\configuration2\builder\
> INIBuilderParametersImpl.java[52:84]
> (blocks) LeftCurly: '{' should be on a new line.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> builder\INIBuilderProperties.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\ex\
> ConfigurationRuntimeException.java[68]
> (regexp) RegexpSingleline: Line has trailing spaces.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\JSONConfiguration.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> JSONConfiguration.java[43:5]
> (javadoc) JavadocVariable: Missing a Javadoc comment.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> JSONConfiguration.java[44:5]
> (javadoc) JavadocVariable: Missing a Javadoc comment.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\tree\
> ImmutableNode.java[106]
> (regexp) RegexpSingleline: Line has trailing spaces.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\tree\
> ImmutableNode.java[114:27]
> (blocks) LeftCurly: '{' should be on a new line.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\tree\
> ImmutableNode.java[117]
> (regexp) RegexpSingleline: Line has trailing spaces.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\tree\
> ImmutableNode.java[666]
> (regexp) RegexpSingleline: Line has trailing spaces.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> XMLConfiguration.java[1169:15]
> (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> XMLConfiguration.java[1210:15]
> (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> XMLConfiguration.java[1212:19]
> (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\
> XMLConfiguration.java[1311:20]
> (whitespace) WhitespaceAround: 'if' is not followed by whitespace.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\XMLListReference.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\XMLListReference.java[45]
> (design) FinalClass: Class XMLListReference should be declared as final.
> [ERROR]
> src\main\java\org\apache\commons\configuration2\YAMLConfiguration.java[0]
> (misc) NewlineAtEndOfFile: File does not end with a newline.
> [WARNING] checkstyle:check violations detected but failOnViolation set to
> false
>
> Gary
>

Re: [LANG] Next release

2017-03-14 Thread Allon Mureinik 
The recent commits 0ba25aa, 4fb393b and 15dcb42 buffed up the CI so several
of these reports (javadoc, checkstyle, rat) are checked per commit, so
reviewing the reports should be easier than previous verisons.

If the project's maintainers can review it, I'd like to get PR #257 in
before the release.
It isn't critical, but it completes the work started in PR #254, and it
would be nice to have it included so that the next version can have a clean
start wrt checkstyle verification.

On Mon, Mar 13, 2017 at 11:22 AM, Benedikt Ritter 
wrote:

> Hello,
>
> I’m going to start work on the next [lang] release tonight. If there is
> anything you would like to add, please do it now. If you want to help me
> with the release process, please review the various project reports and
> make sure they look good. Most importantly make sure RAT is clean and Clirr
> does not show breaking changes.
>
> Thank you!
> Benedikt
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [lang] JUnit imports (closes #247)

2017-03-08 Thread Allon Mureinik 
thanks

On Wed, Mar 8, 2017 at 10:22 AM, Benedikt Ritter <brit...@apache.org> wrote:

> Nice catch!
>
> > Am 28.02.2017 um 23:34 schrieb pascalschumac...@apache.org:
> >
> > Repository: commons-lang
> > Updated Branches:
> >  refs/heads/master 98fa164cd -> c79e6fb8f
> >
> >
> > JUnit imports (closes #247)
> >
> > The junit.framework package has been deprecated, and should no longer
> > be used.
> >
> > This patch fixes the one remaining import from it to statically import
> > org.junit.Assert.assertNull instead.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/
> c79e6fb8
> > Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/c79e6fb8
> > Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/c79e6fb8
> >
> > Branch: refs/heads/master
> > Commit: c79e6fb8f95bb07d2394221b29b75fcc016f4900
> > Parents: 98fa164
> > Author: Allon Mureinik <amure...@redhat.com>
> > Authored: Tue Feb 28 23:17:51 2017 +0200
> > Committer: pascalschumacher <pascalschumac...@gmx.net>
> > Committed: Tue Feb 28 23:33:26 2017 +0100
> >
> > --
> > src/test/java/org/apache/commons/lang3/RandomUtilsTest.java | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > --
> >
> >
> > http://git-wip-us.apache.org/repos/asf/commons-lang/blob/
> c79e6fb8/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
> > --
> > diff --git a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
> b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
> > index ed032e7..ddbaab1 100644
> > --- a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
> > +++ b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
> > @@ -16,10 +16,10 @@
> >  */
> > package org.apache.commons.lang3;
> >
> > -import static junit.framework.TestCase.assertNotNull;
> > import static org.junit.Assert.assertArrayEquals;
> > import static org.junit.Assert.assertEquals;
> > import static org.junit.Assert.assertFalse;
> > +import static org.junit.Assert.assertNotNull;
> > import static org.junit.Assert.assertTrue;
> >
> > import org.junit.Test;
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

[lang] Why have a HashSetvBitSetTest?

2017-02-25 Thread Allon Mureinik 
Hi all,

Building commons-lang locally made me notice HashSetvBitSetTest, which made
me wonder why we need it as a unit test.

I'm guessing it was originally used to support a design decision about
which data structure to use. But having done it, we now have  a relatively
slow test (on my machine - 0.582 seconds out of a total seconds it takes to
mvn test ~ 2.1%), which prints to stdout, and doesn't test any
functionality of the code provided in commons-lang.

Wouldn't it make more sense to @Ignore this test and just run it when a new
jvm/jdk is tested? or even remove it completely?


-Allon