Re: [text] Deprecation of StrMatcher/StrBuilder/StrTokenizer classes

2018-02-12 Thread Gary Gregory 
On Mon, Feb 12, 2018 at 6:54 PM, sebb  wrote:

> On 12 February 2018 at 20:29, Gary Gregory  wrote:
> > On Mon, Feb 12, 2018 at 12:53 PM, Pascal Schumacher <
> > pascalschumac...@gmx.net> wrote:
> >
> >> Hello everybody,
> >>
> >> after further consideration I'm not sure if it is a good idea to
> deprecate
> >> StrMatcher (to convert it to an interface and make the naming match
> >> *StringLookup), as this means that StrBuilder and StrTokenizer also
> have to
> >> be deprecated.
> >>
> >> Not long ago we told users of these classes to migrate from commons-lang
> >> to commons-text. Now they have to migrate again. Seems a bit much imho.
> >>
> >> What do you think?
> >>
> >
> > What this tells me is that the release of commons-text was made in haste
>
> RERO?
>

That's what I am trying to do :-)

Gary


>
> > but with the understanding that one of the goals was migration from
> Commons
> > Lang to Commons Text. Now that we are past that, some sore (IMO) design
> > points are popping up. I just do not think it is a good idea to
> perpetuate
> > the use of the current/old abstract classes. IMO the design should have
> > been based on interfaces to begin with but, I imagine this code came from
> > Commons Configuration or some such place. So we have another case
> > (probably) where the API was copied in bulk which made migration easier
> out
> > of wherever into Commons Lang.
> >
> > Now that Commons Text exists and is its own component, it behooves us to
> > shore up any of the shortcoming from this code that eventually landed
> here
> > and provide a cleaner better API, and interfaces do just that.
> >
> > Gary
> >
> >
> >> Cheers,
> >>
> >> Pascal
> >>
> >>
> >>
> >>
> >>
> >> -
> >> 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: [text] Deprecation of StrMatcher/StrBuilder/StrTokenizer classes

2018-02-12 Thread sebb 
On 12 February 2018 at 20:29, Gary Gregory  wrote:
> On Mon, Feb 12, 2018 at 12:53 PM, Pascal Schumacher <
> pascalschumac...@gmx.net> wrote:
>
>> Hello everybody,
>>
>> after further consideration I'm not sure if it is a good idea to deprecate
>> StrMatcher (to convert it to an interface and make the naming match
>> *StringLookup), as this means that StrBuilder and StrTokenizer also have to
>> be deprecated.
>>
>> Not long ago we told users of these classes to migrate from commons-lang
>> to commons-text. Now they have to migrate again. Seems a bit much imho.
>>
>> What do you think?
>>
>
> What this tells me is that the release of commons-text was made in haste

RERO?

> but with the understanding that one of the goals was migration from Commons
> Lang to Commons Text. Now that we are past that, some sore (IMO) design
> points are popping up. I just do not think it is a good idea to perpetuate
> the use of the current/old abstract classes. IMO the design should have
> been based on interfaces to begin with but, I imagine this code came from
> Commons Configuration or some such place. So we have another case
> (probably) where the API was copied in bulk which made migration easier out
> of wherever into Commons Lang.
>
> Now that Commons Text exists and is its own component, it behooves us to
> shore up any of the shortcoming from this code that eventually landed here
> and provide a cleaner better API, and interfaces do just that.
>
> Gary
>
>
>> Cheers,
>>
>> Pascal
>>
>>
>>
>>
>>
>> -
>> 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: [CSV][POLL] How to provide mutable records

2018-02-12 Thread Gary Gregory 
On Fri, Feb 9, 2018 at 10:05 AM, Stian Soiland-Reyes 
wrote:

> On Fri, 25 Aug 2017 19:19:58 +0100, Stian Soiland-Reyes 
> wrote:
> > This came up also for commons rdf where we also have everything
> immutable,
> > which I think is a good principle to keep for modern Java 8 programming.
> >
> > So you need a mutator function like in (4) that either returns a new
> > immutable (but changed) CSVRecord; or alternatively a different
> > MutableCSVRecord that can then be built/frozen to a CSVRecord. (These can
> > then share a common accessor interface for the passive functions)
>
> Picking up this thread to consider this for CSV 1.6.
>
> Not quite as elegant as above, but I made
> some mutator functions withValue() in
> https://github.com/apache/commons-csv/pull/25
>
>
> for (CSVRecord r : csvparser) {
>   CSVRecord rSoup = r.withValue(4, "soup")
>  .withValue(5, "fish");
>   // original r is untouched and can be used again
>   CSVRecord rBeans = r.withValue(3, "beans");
>
>   List list;
>   // Each now different
>   someList.add(r);
>   someList.add(rSoup);
>   someList.add(rBeans);
>
>   // worried someone might touch your beans?
>   consumeCSVRecord(rBeans.immutable())
> }
>
> It's not clever enough (yet!) to resize the underlying array if you try to
> go
> outside the existing columns. The existing parser seems to detect column
> number
> (and hence record array size) per line so this might be weird for some
> inconsistent CSV files.
>
>
>
> Comments and changes on CSV-216 branch welcome.
>

Hi Stian,

I've not had time to review this yet but I hope to get to it sometimes this
week.

Gary

>
> --
> Stian Soiland-Reyes
> The University of Manchester
> http://www.esciencelab.org.uk/
> http://orcid.org/-0001-9842-9718
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

[GitHub] commons-rdf issue #43: COMMONSRDF-49: Make AbstractRDFParser serializable

2018-02-12 Thread stain 
Github user stain commented on the issue:

https://github.com/apache/commons-rdf/pull/43
  
Picking up this - @ajs6f do you still think we should proceed along those 
lines? I am also reluctant to making the abstract factory (builder) 
serializable, but I can see the reasoning, particularly if you want to use this 
in Hadoop or something where you have a pre-made parser builder and then tell a 
different node to run it.

One thing I feel I need to check more is that there is no reading of the 
now-usually-null fields beyond the getters - I might rename them to make that 
clear.

I have put this PR into upstream branch COMMONSRDF-49 we can contribute to.


---

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

[GitHub] commons-rdf pull request #48: Cleanup in commons-rdf-rdf4j to close PMD and ...

2018-02-12 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/commons-rdf/pull/48


---

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

[GitHub] commons-rdf pull request #50: Cleanup for PMD warnings in -rdf-api

2018-02-12 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/commons-rdf/pull/50


---

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

[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...

2018-02-12 Thread stain 
Github user stain commented on a diff in the pull request:

https://github.com/apache/commons-rdf/pull/49#discussion_r167732171
  
--- Diff: 
commons-rdf-simple/src/main/java/org/apache/commons/rdf/simple/experimental/AbstractRDFParser.java
 ---
@@ -58,8 +60,12 @@
  */
 public abstract class AbstractRDFParser> 
implements RDFParser, Cloneable {
 
-public static final ThreadGroup threadGroup = new ThreadGroup("Commons 
RDF parsers");
-private static final ExecutorService threadpool = 
Executors.newCachedThreadPool(r -> new Thread(threadGroup, r));
+public static final AtomicInteger threadCount = new AtomicInteger();
+private static Thread newThread(Runnable r) {
--- End diff --

I disagree on this style change, the previous code was much cleaner. Why is 
a method-reference on 4 lines that is only used once better than a very small 
lambda?


---

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

[GitHub] commons-rdf pull request #52: Adding convenient Dataset and DatasetGraph con...

2018-02-12 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/commons-rdf/pull/52


---

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

Re: [All] Convention for "courtesy" codes?

2018-02-12 Thread Gilles 

On Sat, 10 Feb 2018 20:50:26 +0100, Stefan Bodewig wrote:

On 2018-02-10, Gilles wrote:


On Sat, 10 Feb 2018 08:08:12 -0700, Gary Gregory wrote:
If a Java package and artifact ID contain "internal" or "private" 
in

the
name, that would be pretty clear.



Do you suggest that, say, the benchmarking codes in
"commons-rng-jmh" should be located in a top package
named "o.a.c.rng.jmh.internal"?


Gary's response likely stems from me misunderstanding what you asked
about. I overlooked you said "modules" and assumed you were talking
about parts of an artifact which otherwise should evolve in a 
backwards

compatible way.

If a whole artifact is not considered something that is there for 
public
consumption as an API then I'd just say so (inside the POM, in 
javadocs,

on the website ...) and not care for backwards compatibility at all.

IMHO we don't need any rules for something like this, proper
documentaton should be enough.


I'll then also assume that the layout of the maven project
can be chosen freely for those packages/modules.

For clarity's sake, in the case of "Commons RNG", I intend
to move all "non-library" codes to sub-modules of module
"commons-rng-examples".

Any objections?

Gilles



Stefan




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

Re: [text] Adapt the Log4j 2 Interpolator to [text]

2018-02-12 Thread Gary Gregory 
On Mon, Feb 12, 2018 at 12:53 PM, Gary Gregory 
wrote:

>
>
> On Mon, Feb 12, 2018 at 12:30 PM, Pascal Schumacher <
> pascalschumac...@gmx.net> wrote:
>
>> Am 12.02.2018 um 18:52 schrieb Gary Gregory:
>>
>>> I agree 100% and will proceed. I thought about it overnight and it does
>>> not
>>> make sense to leave a mix of abstract classes and interfaces in
>>> StrSubstitutor.
>>>
>>
>> +1, but
>>
>> please revert "Update actual Checkstyle from 6.19 to 8.8.", as Checkstyle
>> 7+ requries Java 8+.
>>
>
> Done.
>
>
>>
>> and please fix the findbugs violation:
>>
>> [INFO] --- findbugs-maven-plugin:3.0.5:check(default-cli)@
>> commons-text---
>> [INFO] BugInstance size is 1
>> [INFO] Error size is 0
>> [INFO] Total bugs: 1
>> [INFO] org.apache.commons.text.StringTokenizer.clone() does not call
>> super.clone() [org.apache.commons.text.StringTokenizer] At
>> StringTokenizer.java:[lines 1138-1140] CN_IDIOM_NO_SUPER_CALL
>>
>
> But super.clone() is called, from another method...
>

And findbugs does not complain about the same code in StrTokenizer. What?

Gary


>
> Gary
>
>
>>
>> to fix the travis build.
>>
>> Thanks,
>> Pascal
>>
>
>

Re: [text] Deprecation of StrMatcher/StrBuilder/StrTokenizer classes

2018-02-12 Thread Gary Gregory 
On Mon, Feb 12, 2018 at 12:53 PM, Pascal Schumacher <
pascalschumac...@gmx.net> wrote:

> Hello everybody,
>
> after further consideration I'm not sure if it is a good idea to deprecate
> StrMatcher (to convert it to an interface and make the naming match
> *StringLookup), as this means that StrBuilder and StrTokenizer also have to
> be deprecated.
>
> Not long ago we told users of these classes to migrate from commons-lang
> to commons-text. Now they have to migrate again. Seems a bit much imho.
>
> What do you think?
>

What this tells me is that the release of commons-text was made in haste
but with the understanding that one of the goals was migration from Commons
Lang to Commons Text. Now that we are past that, some sore (IMO) design
points are popping up. I just do not think it is a good idea to perpetuate
the use of the current/old abstract classes. IMO the design should have
been based on interfaces to begin with but, I imagine this code came from
Commons Configuration or some such place. So we have another case
(probably) where the API was copied in bulk which made migration easier out
of wherever into Commons Lang.

Now that Commons Text exists and is its own component, it behooves us to
shore up any of the shortcoming from this code that eventually landed here
and provide a cleaner better API, and interfaces do just that.

Gary


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

Re: [text] Update from Java 7 to Java 8?

2018-02-12 Thread Pascal Schumacher 

What about now? :-)

Am 12.02.2018 um 20:53 schrieb Gary Gregory:

When?

Gary




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

[text] Update from Java 7 to Java 8?

2018-02-12 Thread Gary Gregory 
When?

Gary

Re: [text] Adapt the Log4j 2 Interpolator to [text]

2018-02-12 Thread Gary Gregory 
On Mon, Feb 12, 2018 at 12:30 PM, Pascal Schumacher <
pascalschumac...@gmx.net> wrote:

> Am 12.02.2018 um 18:52 schrieb Gary Gregory:
>
>> I agree 100% and will proceed. I thought about it overnight and it does
>> not
>> make sense to leave a mix of abstract classes and interfaces in
>> StrSubstitutor.
>>
>
> +1, but
>
> please revert "Update actual Checkstyle from 6.19 to 8.8.", as Checkstyle
> 7+ requries Java 8+.
>

Done.


>
> and please fix the findbugs violation:
>
> [INFO] --- findbugs-maven-plugin:3.0.5:check(default-cli)@ commons-text---
> [INFO] BugInstance size is 1
> [INFO] Error size is 0
> [INFO] Total bugs: 1
> [INFO] org.apache.commons.text.StringTokenizer.clone() does not call
> super.clone() [org.apache.commons.text.StringTokenizer] At
> StringTokenizer.java:[lines 1138-1140] CN_IDIOM_NO_SUPER_CALL
>

But super.clone() is called, from another method...

Gary


>
> to fix the travis build.
>
> Thanks,
> Pascal
>

[text] Deprecation of StrMatcher/StrBuilder/StrTokenizer classes

2018-02-12 Thread Pascal Schumacher 

Hello everybody,

after further consideration I'm not sure if it is a good idea to 
deprecate StrMatcher (to convert it to an interface and make the naming 
match *StringLookup), as this means that StrBuilder and StrTokenizer 
also have to be deprecated.


Not long ago we told users of these classes to migrate from commons-lang 
to commons-text. Now they have to migrate again. Seems a bit much imho.


What do you think?

Cheers,

Pascal





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

Re: [text] Adapt the Log4j 2 Interpolator to [text]

2018-02-12 Thread Pascal Schumacher 

Am 12.02.2018 um 18:52 schrieb Gary Gregory:

I agree 100% and will proceed. I thought about it overnight and it does not
make sense to leave a mix of abstract classes and interfaces in
StrSubstitutor.


+1, but

please revert "Update actual Checkstyle from 6.19 to 8.8.", as 
Checkstyle 7+ requries Java 8+.


and please fix the findbugs violation:

[INFO] --- findbugs-maven-plugin:3.0.5:check(default-cli)@ commons-text---
[INFO] BugInstance size is 1
[INFO] Error size is 0
[INFO] Total bugs: 1
[INFO] org.apache.commons.text.StringTokenizer.clone() does not call 
super.clone() [org.apache.commons.text.StringTokenizer] At 
StringTokenizer.java:[lines 1138-1140] CN_IDIOM_NO_SUPER_CALL


to fix the travis build.

Thanks,
Pascal

Re: [text] Adapt the Log4j 2 Interpolator to [text]

2018-02-12 Thread Gary Gregory 
On Sun, Feb 11, 2018 at 8:12 PM, Chas Honton  wrote:

> Definitely create interfaces.
>

I agree 100% and will proceed. I thought about it overnight and it does not
make sense to leave a mix of abstract classes and interfaces in
StrSubstitutor.

Gary


>
> Chas
>
> > On Feb 11, 2018, at 1:57 PM, Gary Gregory 
> wrote:
> >
> > On Sun, Feb 11, 2018 at 2:46 PM, Gary Gregory 
> > wrote:
> >
> >>
> >>
> >> On Sun, Feb 11, 2018 at 12:05 PM, Pascal Schumacher <
> >> pascalschumac...@gmx.net> wrote:
> >>
>  Am 11.02.2018 um 19:24 schrieb Gary Gregory:
> 
>  I'd like a code review and then a release of 1.3. Right now we only
>  depend
>  on java.base and Commons Lang, so let's keep it that way for 1.3 I
> think.
> 
> >>> My comments:
> >>>
> >>
> >> Thank you for the code review!
> >>
> >>
> >>> - Given "TEXT-80: StrLookup API confusing generic type parameter" I
> think
> >>> we should deprecate the old StrLookup class and mark it for removal in
> >>> commons-text 2.0.
> >>>
> >> +1 and will do.
> >>
> >>
> >>> - DateStringLookup: should we use FastDateFormat?
> >>>
> >>
> >> I overlooked that one, I'll look into it.
> >>
> >>
> >>> - AbstractStringLookup: empty class, I would therefore remove it.
> >>>
> >>
> >> I like to have it around for now, it is package private anyway. We can
> >> remove it before 1.3 if it stays empty. At one point I have an
> >> isEmpty(String) method in there before I realized that Commons Text
> depends
> >> on Commons Lang which provides the same service in
> >> StringUtils.isEmpty(String).
> >>
> >>
> >>> - StringLookupFactory: should this be a static factory, to make it
> easier
> >>> to use?
> >>
> >>
> >> I am leaving room for configuring these things later so I feel that
> doing
> >> .INSTANCE is a small price to pay but I hear you.
> >>
> >
> > Oh, and consider this alternative:
> >
> > - Create StringLookup (already there) and StringSubstitutor
> > - Deprecate StrLookup and StrSubstitutor and leave as is from 1.2
> > - ALSO deprecate StrMatcher which is a class and introduce a
> StringMatcher
> > _interface_.
> >
> > The idea here is to avoid having to do this a second time later. Right
> now
> > StrLookup and StrMatcher are classes, there are no interfaces. The
> question
> > is: Should we just redo the whole thing based on interfaces now. As of
> now
> > in master, we have a 50/50 situation with StrSubstituor supporting both
> > StrLookup and StringLookup AND using StrMatcher (a class, not an
> interface).
> >
> > Thoughts?
> >
> > Gary
> >
> >
> >> Gary
> >>
> >>
> >>>
> >>>
> >>> (I almost added Log4j's JNDI lookup but I know that will not work on
>  Android so I'd like to leave stuff like that for later, maybe in a
>  different module.)
> 
> >>> +1 for leaving it out
> >>>
> >>> -Pascal
> >>>
> >>>
> >>> -
> >>> 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
>
>