[GitHub] commons-lang issue #391: Adding junits for JsonToStringStyle

2018-12-13 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/391
  
This project is maintained by volunteers @RahulNagekar. Please be patient 
as maintainers can be busy or under pressure with other issues, work, life, etc.


---


[GitHub] commons-lang issue #380: fix javadoc typo

2018-11-02 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/380
  
Merged. Many thanks @oorijsonar 


---


[GitHub] commons-lang issue #379: add tests for use ImmutableTriple as key in java.ut...

2018-10-30 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/379
  
@apirom9 does this pull request number #379 supersedes yours previous one, 
number  #378? It looks to me like the tests from #378 are present in this pull 
request too? If so then we should close #378 to avoid confusion when reviewing 
& merging.

Thanks
Bruno


---


[GitHub] commons-lang pull request #356: Move combine.children to manifestEntries in ...

2018-09-08 Thread kinow
GitHub user kinow opened a pull request:

https://github.com/apache/commons-lang/pull/356

Move combine.children to manifestEntries in order to have right entries in 
MANIFEST.MF

Hi,

While working on Commons Imaging 1.0-alpha release candidate, I noticed 
that after copying most of the entries missing from Commons Lang's `pom.xml`, I 
couldn't pass the [step](https://commons.apache.org/releases/prepare.html) that 
asks to check for `X-Compile-Source-JDK` in `MANIFEST.MF` .

After some troubleshooting (and updating my Maven for the just-in-case), I 
realized that the combine.child seemed to result in `manifestEntries` child 
node being replaced.

But moving the `combine.child` to the `manifestEntries` made more sense, as 
that's the key that we want Maven to append parent+child.

Tested locally in Commons Imaging, and also in Commons Lang. After this 
change, I can successfully see the entries in the Lang's new MANIFEST.MF 
generated locally.

```
...
Bundle-Version: 3.9.0.SNAPSHOT
X-Compile-Target-JDK: 1.8
Implementation-Build: master@r6dc3a6db11d7e63c960ccc6cf48aa15d6f00e903
 ; 2018-09-08 10:35:11+
X-Compile-Source-JDK: 1.8
Created-By: Apache Maven Bundle Plugin
Build-Jdk: 1.8.0_181
...
```

Cheers
Bruno

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kinow/commons-lang fix-manifest-entries

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-lang/pull/356.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #356


commit 790655c1f82645d48a53456388304bf754d5470e
Author: Bruno P. Kinoshita 
Date:   2018-09-08T10:42:13Z

Move combine.children to manifestEntries in order to have right entries in 
MANIFEST.MF




---


[GitHub] commons-lang issue #340: [LANG-1406] StringIndexOutOfBoundsException in Stri...

2018-09-05 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/340
  
Good progress so far @HiuKwok . Understanding the problem well is a good 
first step to solve it 👍 my next development cycle for Apache will probably 
be for a release for Apache Commons Imaging, then after that Lang+Text. So will 
try to help here if you haven't found a way to solve it yet.


---


[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...

2018-09-05 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/353
  
I think so @smoyer64 , but not meaning giving up on the task, just split it 
into smaller changes, or a complete change but with less distractions from the 
main work. Alternatively, you can update the PR with some base work that you 
think might be useful for the next iterations. Up to you :-)


---


[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...

2018-09-05 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/353
  
>In general it's hard to review gigantic change sets which have been 
created automatically, so I'd welcome an approach where we migrate one test 
case after another.

+1, and bonus points if we refactor some of the old test code, or maybe 
even cover extra parts of the code :-) :tada


---


[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...

2018-09-04 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/353
  
@smoyer64 it looks interesting! Still have to update some projects at work 
as well, so keen to investigate it later. But, do you know if there's a way to 
disable some formatting changes? I noticed several files have diffs consisting 
mainly of blank lines being removed... without those changes I think it would 
be much easier to review it. Thanks!


---


[GitHub] commons-lang issue #342: LANG-1411: Add empty checks to ObjectUtils

2018-08-17 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/342
  
Hi @nictas , thanks for your contribution. The code looks really good, well 
commented and formatted with unit tests. :clap: :clap: 

And you seem to have a valid use case for it. I think I could even get used 
to calling this method in some of my projects.

Leaving my +1 here, but leaving it for others to review. Would be 
especially good someone with more time to confirm we don't have anything 
similar, or that could be chained, etc, I think. 

Thank you again
Bruno


---


[GitHub] commons-lang pull request #340: [LANG-1406] StringIndexOutOfBoundsException ...

2018-08-14 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/340#discussion_r209907553
  
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -5596,8 +5596,8 @@ private static String replace(final String text, 
String searchString, final Stri
  }
  String searchText = text;
  if (ignoreCase) {
- searchText = text.toLowerCase();
- searchString = searchString.toLowerCase();
+ searchText = text.toUpperCase();
+ searchString = searchString.toUpperCase();
--- End diff --

Yeah, a very slippery problem. We still have the option to simply document 
that the method does not work well with unicode strings. 

But first I would like to spend at least a few hours with paper and pencil 
(and eraser, because this could take a bit till I give up or find a possible 
way around it), and perhaps even check in the mailing list if other devs have 
any idea.

I think you found a very interesting problem (*)! Keep the ideas coming if 
you have any on how to solve this issue :+1: 

_* if I had more time, I would possibly either go through other methods 
checking for that or, just try some fuzzifier approach to test the whole 
project ! Not aware of any static or dynamic analysis tool that does that_


---


[GitHub] commons-lang pull request #340: [LANG-1406] StringIndexOutOfBoundsException ...

2018-08-13 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/340#discussion_r209587514
  
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -5596,8 +5596,8 @@ private static String replace(final String text, 
String searchString, final Stri
  }
  String searchText = text;
  if (ignoreCase) {
- searchText = text.toLowerCase();
- searchString = searchString.toLowerCase();
+ searchText = text.toUpperCase();
+ searchString = searchString.toUpperCase();
--- End diff --

Oohh, great testing @HiuKwok ! Thanks for sharing here.

>Just a quick question what you mean by remove the length( ) mean? Would 
you mind to specify more on that?

Sure. I think there could be a possibility to fix the issue by addressing 
how the length of the lower'ed/upper'ed text is used 
https://github.com/apache/commons-lang/blob/590f90889bf61a5570bd98b78e73410a07d7410b/src/main/java/org/apache/commons/lang3/StringUtils.java#L5603

So maybe there could be another way to work around the way we use the 
strings lengths, and avoid the exception.


---


[GitHub] commons-lang pull request #340: [LANG-1406] StringIndexOutOfBoundsException ...

2018-08-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/340#discussion_r208851249
  
--- Diff: src/main/java/org/apache/commons/lang3/StringUtils.java ---
@@ -5596,8 +5596,8 @@ private static String replace(final String text, 
String searchString, final Stri
  }
  String searchText = text;
  if (ignoreCase) {
- searchText = text.toLowerCase();
- searchString = searchString.toLowerCase();
+ searchText = text.toUpperCase();
+ searchString = searchString.toUpperCase();
--- End diff --

Just leaving a comment here too to have a review here in GitHub. While your 
example works, as the character is considered already in upper case, the 
reverse case would still fail after changing from `toLowerCase` to 
`toUpperCase`. So I think we should find another solution or update the 
documentation stating how the code works with unicode.


---


[GitHub] commons-lang issue #340: [LANG-1406] StringIndexOutOfBoundsException in Stri...

2018-08-09 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/340
  
Oh, that does make sense now. So the first visible character we see is the 
["Latin Capital Letter I with Dot Above"](https://unicode-table.com/en/#0130) 
(see also [this other 
link](https://en.wikipedia.org/wiki/Dotted_and_dotless_I)), and the second an 
`x`. And doing `toUpperCase()` simply won't change it as it's considered 
already upper case.

When doing a `toLowerCase`, it gets translated into two visible characters. 
The second is the normal `x`. While the first contains two codepoints. I tested 
in Python, and got the lower case `i` (`print(u"\u0069")`) followed by a 
character invisible by itself (`print (u"\u0307")`).

The special/invisible character, is visible when coming after certain 
letters.

```python
>>> print(u"\u0307")

>>> print(u"\u0069\u0307")
i̇
>>> print(u"\u0068\u0307")
ḣ
>>> print(u"\u0067\u0307")
ġ
>>> print(u"\u0067\u0307")
```

When we get these invisible characters, as we have one code point more, the 
length returned is not 2, but 3. Resulting in exception in this issue.

I don't believe the fix here would fix the reverse case, where we had a 
lower case, single codepoint, unicode; that would be represented by a two code 
codepoint. The exception could happen again (I haven't investigated whether 
such case exist, but I'm assuming there could be such case - if not now, maybe 
a character could still be added in future editions).

What do you think @HiuKwok ? Any suggestions? I'm not sure if there's any 
easy way to fix this case, except by adding a note to the documentation saying 
that the method is not intended to be used with unicode strings, as it doesn't 
handle supplementary characters well. Or maybe we could try to remove the 
`length()` call around the `StringBuilder`'s near the end of the method...


---


[GitHub] commons-lang issue #340: [LANG-1406] StringIndexOutOfBoundsException in Stri...

2018-08-08 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/340
  
I'm surprised by this bug. Had no idea something like this could happen. 
Will debug later and see if I can understand why that happens (might have to 
train my brain to default to always use uppercase instead of lowercase?). 
Thanks for the pull request, we will review the code and if everything looks OK 
a committer will merge it.


---


[GitHub] commons-lang issue #275: WIP LANG-1339: replace java.beans.PropertyListener ...

2018-07-16 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/275
  
Back to WIP. Please avoid merging it for now. Gilles noted in the mailing 
list that the Observable/Observer classes have been deprecated in Java 9. So we 
will probably go with the deprecation/removal path instead.

Link to the discussion in the mailing list: 
https://markmail.org/message/efy52lul5pzbznut


---


[GitHub] commons-lang issue #334: Code refactoring and cleaning

2018-06-23 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/334
  
Thanks @PascalSchumacher for confirming it's actually backward compatible 
(#TodayILearned - and hopefully will remember it).

@ingvarc , I agree on @aaabramov regarding the `+1`. When I read it, I 
first thought that was a `+= 1`, and then re-read it and understood his 
concern. Might be worth simplifying it. If you'd like to keep the commits 
simpler (which is great initiative, thanks!) you could try rebasing commits and 
push+force to your branch.

Good points @stokito. Not sure about branch prediction after the spectre 
bug. I think one of the mitigations involved disabling branch prediction. 
Though I think micro-benchmarking isn't priority in lang.

>Here developer sees a "happy path" first and only then he or she sees 
exception situation handling.
This is more natural to understand. As I remember this style was mentioned 
in Complete Code book.

Code Complete is one of my favourite books. For me, that code could be:

```java
private static void addProcessor(final String key, final Processor 
processor) {
if (ARCH_TO_PROCESSOR.containsKey(key)) {
throw new IllegalStateException("Key " + key + " already exists 
in processor map");
}
ARCH_TO_PROCESSOR.put(key, processor);
}
```

I think maybe developers would read the first `if` as a validation block. 
Then the normal behaviour, without the else (I think omitting the else in cases 
like this is also in Code Complete... or Pragmatic Programmer? Can't recall).

I think the other changes like removing class name, inverting some `if` 
conditions, and removing the exceptions not thrown look good, and the code 
readability keeps OK. Great contribution @ingvarc , thanks!



---


[GitHub] commons-lang issue #318: clean code

2018-06-23 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/318
  
@JiangYongKang, thanks for your contribution.

Even though the behaviour is the same, in my opinion, the other code is 
easier to read, and fewer code doesn't necessarily mean better or more clean 
code.

Checking if a number if even or odd, for example, can be done with bit 
operation, or with a `if/else`. While the former is shorter, not all developers 
immediately understand it, nor know how to maintain/change it.


---


[GitHub] commons-lang issue #334: Code refactoring and cleaning

2018-06-20 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/334
  
I'm fine either way (I believe even some classes in the JDK have some 
[unchecked 
exceptions](http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/share/classes/java/util/LinkedList.java)
 declared?)

But I suspect this change is not backward compatible. Can't locate that 
documentation from Oracle explaining what's a backward change or not, but quite 
sure this one is.


---


[GitHub] commons-lang issue #275: LANG-1339: replace java.beans.PropertyListener by j...

2018-06-09 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/275
  
Posted to the mailing list and will wait to see if there is any further 
feedback. Otherwise I will merge it in the next days. Thank you @jodastephen ! 


---


[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...

2018-06-09 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/275
  
@jodastephen done. Kept the existing classes, but added new ones where the 
only modification is replacing `java.beans` by `java.util` equivalent classes.

Existing classes were annotated with `@deprecated` with a comment pointing 
to the new class.

WDTY? I'd like to sort it out as soon as possible to sort out the issue 
with dependencies & Java 9 in lang.

Thanks!
Bruno

ps: old code was removed from commit line. Moved instead to a branch at 
https://github.com/kinow/commons-lang/tree/LANG-1339-old, just in case we need 
to compare it or someone wants to see what it was before


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Well, no, but I quite like your example. Will give it a try to see how it 
would look like  without looking at the code (mobile right now), my only 
concern would be using try-with-resource when we don't open a real resource nor 
throw any exception (can't recall that part).

But I liked the start static method in special. WDYT @ottobackwards?


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
@ottobackwards I'm +1 for using generics, as well as the fluent API 
suggested by Gilles just now. But I'd have to look at the code to see if that's 
doable. I think you are probably the best to take a look if that'd a) be a good 
idea, b) be doable, or c) argument why not.

The generics I suspect might be easier to achieve. Fluent API's are 
normally useful, but easily to be overdone IMO.


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Oh, and you have a pending comment in your JIRA ticket from another 
reviewer :-)


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
I would run something like `git rebase -i HEAD~123` where 123 is the number 
of commits I'm going back. Then squash the commits into a single one, 
discarding commit messages. Finally, would use a single commit message like 
"LANG-1373: The description here...". And then `git fetch --all` followed by a 
`git rebase origin/master` to make sure it's at the most recent change in the 
remote repository as well. If no merge conflicts, you can simply `push -f` to 
your branch, and that should do it.

In case you are concerned about losing your changes, just do from your 
branch a `git checkout -b backup-1373` or some other name, `git checkout -` to 
go back to your branch, and give it a try (or do it in the other branch, 
whichever you prefer)


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-20 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
E-mail sent. See the latest messages in the Commons Developers mailing 
list, with the tag [lang]. There should be one from me about this PR and your 
JIRA ticket, asking for more reviews.

Thanks for being so patient @ottobackwards !!

Bruno


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-02-20 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Pull request looks good. Toyed with the code a bit 
(https://gist.github.com/kinow/c0873daf069c2e64bc69d6305400e610), and I like 
the current design.

@ottobackwards let's ask in the ML if anybody else would like to review it, 
and if nobody replies by the weekend, consider it merged. During the merge, I'd 
like to squash the commits to keep the commit history/tree simpler. Would you 
mind squashing the commits and updating the pull request to one single commit 
(or a few if you think it's necessary), please?

Cheers
B


---


[GitHub] commons-lang pull request #311: LANG-1373 Stopwatch based capability for nes...

2018-01-27 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/311#discussion_r164283339
  
--- Diff: src/main/java/org/apache/commons/lang3/time/TimingRecordNode.java 
---
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.lang3.time;
+
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * The tree node to track time and children.
+ * The {@code StopWatch} class is used for timings
+ */
+public class TimingRecordNode {
+
+/**
+ * The format String for creating paths.
+ */
+private static final String PATH_FMT = "%s/%s";
+
+/**
+ * This nodes parent's path.
+ */
+private String parentTimingPath;
+
+/**
+ * The name of this node.
+ */
+private String timingName;
+
+/**
+ * The tags associated with this timing.
+ */
+private String[] tags;
+
+/**
+ * The child nodes of this node.
+ */
+private List children = new LinkedList<>();
+
+/**
+ * The {@code StopWatch} for this node.
+ */
+private StopWatch stopWatch = new StopWatch();
+
+/**
+ * 
+ * Constructor.
+ * 
+ * 
+ * Creates a new TimingRecordNode for a given parent name, with a 
given name.
+ * 
+ *
+ * @param parentTimingPath the path of the parent, may be null
+ * @param timingName the name of the timing
+ * @param tags the tags to associate with this timing
+ * @throws IllegalArgumentException if the timingName is null or empty.
+ */
+public TimingRecordNode(String parentTimingPath, String timingName, 
String... tags) {
+if (StringUtils.isEmpty(timingName)) {
+throw new IllegalArgumentException("Argument name is missing");
+}
+this.timingName = timingName;
+
+if (StringUtils.isNotEmpty(parentTimingPath)) {
+this.parentTimingPath = parentTimingPath;
+}
+
+this.tags = tags;
+}
+
+/**
+ * Returns the node's parent's path.
+ * The parent node path may be null
+ *
+ * @return the parent node path
+ */
+public String getParentPath() {
+return parentTimingPath;
+}
+
+/**
+ * Returns the node's timing name.
+ *
+ * @return the node timing name
+ */
+public String getTimingName() {
+return timingName;
+}
+
+/**
+ * Return if the node's StopWatch is running.
+ *
+ * @return true if it is running, false if not
+ */
+public boolean isRunning() {
+return stopWatch.isStarted();
+}
+
+/**
+ * Starts the StopWatch.
+ */
+public void start() {
+if (!stopWatch.isStarted()) {
+stopWatch.start();
+}
+}
+
+/**
+ * 
+ * Stops the StopWatch.
+ * 
+ * 
+ * If this node has running children, an {@code IllegalStateException} 
will result.
+ * 
+ *
+ * @throws IllegalStateException if stop is called on a node with 
running children
+ */
+public void stop() {
+for (TimingRecordNode child : children) {
+if (child.isRunning()) {
+throw new IllegalStateException("Cannot stop a timing with 
running children");
+}
+}
+stopWatch.stop();
+}
+
+/**
+ * Returns the {@code StopWatch} for this node.
+ *
+ * @return {@code StopWatch}
+ */
+public 

[GitHub] commons-lang pull request #311: LANG-1373 Stopwatch based capability for nes...

2018-01-27 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/311#discussion_r164283366
  
--- Diff: src/main/java/org/apache/commons/lang3/time/TimingRecordNode.java 
---
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.lang3.time;
+
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * The tree node to track time and children.
+ * The {@code StopWatch} class is used for timings
+ */
+public class TimingRecordNode {
+
+/**
+ * The format String for creating paths.
+ */
+private static final String PATH_FMT = "%s/%s";
+
+/**
+ * This nodes parent's path.
+ */
+private String parentTimingPath;
+
+/**
+ * The name of this node.
+ */
+private String timingName;
+
+/**
+ * The tags associated with this timing.
+ */
+private String[] tags;
+
+/**
+ * The child nodes of this node.
+ */
+private List children = new LinkedList<>();
--- End diff --

Would it make much difference if we used an `ArrayList` here? We seem to 
`#add` only in `createChild()`, and not sure if we are using any head/tail 
operation, nor inserting with indexes. So maybe having an `ArrayList` would 
give us the same functionality for less memory?


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-27 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
I was the one who was not clear enough :-) sorry


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-27 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Sorry, what I meant was that indentation in the lang is done with 4 spaces. 
Added an example to another class. Hope that makes sense.


---


[GitHub] commons-lang pull request #311: LANG-1373 Stopwatch based capability for nes...

2018-01-27 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/311#discussion_r164282304
  
--- Diff: src/main/java/org/apache/commons/lang3/time/StackWatch.java ---
@@ -0,0 +1,329 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.lang3.time;
+
+import java.util.Deque;
+import java.util.LinkedList;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * 
+ * The {@code StackWatch}, provides a wrapper around the {@code StopWatch} 
for creating multiple and
+ * possibly nested named timings.
+ * 
+ * 
+ * While the {@code StopWatch} provides functionality to time the length 
of operations, there is no
+ * context or name to go with the time tracked. It is also not possible to 
time nested calls with
+ * the {@code StopWatch}.
+ * 
+ * 
+ * {@code StackWatch} provides that functionality, allowing successive 
calls to {@link StackWatch#startTiming(String, String...)} to track
+ * nested calls.
+ * 
+ * 
+ * Each start provides a timing name and a parent timing name, thus 
providing context to the timing.
+ * 
+ * 
+ * At the end of a timing 'run', a visitor interface provides the ability 
to visit all the timing
+ * 'nodes' and capture their output, including the level of the call if 
nested.
+ * 
+ * 
+ * The {@code TimeRecordNodes} provide a tree structure in support of 
nesting.
+ * A {@code Deque} is use to track the current time node.
+ * 
+ *
+ * 
+ *   {@code
+ *private void outerFunction() {
+ *  try {
+ *StackWatch watch = new StackWatch("OuterFunction");
+ *watch.start();
+ *functionOne();
+ *watch.stop();
+ *watch.visit(new TimingRecordNodeVisitor() {
+ *  {@literal @}Override
+ *  public void visitRecord(int level, TimingRecordNode node) {
+ *...
+ *  }
+ *});
+ *  } catch (Exception e){}
+ *}
+ *private void functionOne(StackWatch watch) throws Exception {
+ *  watch.startTiming("One", "OneFunc");
+ *  functionOneOne(watch);
+ *  watch.stopTiming();
+ *}
+ *
+ *private void functionOneOne(StackWatch watch) throws Exception {
+ *  watch.startTiming("OneOne", "OneFunc");
+ *  functionOneTwo(watch);
+ *  watch.stopTiming();
+ *}
+ *
+ *private void functionOneTwo(StackWatch watch) throws Exception {
+ *  watch.startTiming("OneTwo", "OneFunc");
+ *  watch.stopTiming();
+ *}
+ *   }
+ * 
+ *
+ *
+ * 
+ * This class is not thread safe, and is meant to track timings across 
multiple calls on the same
+ * thread
+ * 
+ */
+public class StackWatch {
+
+  /**
+   * The default name for the root level timing if not provided
+   */
+  public static final String DEFAULT_ROOT_NAME = "ROOT_TIMING";
--- End diff --

Here is needs to be four spaced as well. You can look at another file and 
compare that (e.g. 
https://github.com/apache/commons-lang/blob/f50ec5e608286b0c48d6b9b4c792352de8353804/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java#L58).


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Perfect. So maybe it is not enforcing the 4 spaces? Haven't looked at the 
xml settings we have in lang in a while.

So if you fixed the 2/4 spaces, we won't have any other nit-picks like 
this, and be able to focus on the feature. Especially since you added good 
docs, unit cases, and even a practical use case :-)


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Oh, good point. There's the 
[CONTRIBUTING.md](https://github.com/apache/commons-lang/blob/master/CONTRIBUTING.md).
 There are several links in that doc too, apologize for not having a shorter 
version.

The tabs/spaces is mentioned at the first link under *Additional 
Resources*, but there are heaps of other useful information in there. Hopefully 
not too different from other ASF projects, so you can probably filter through 
90% or more of the contents in those files, and just recognize one of two 
differences in the way [lang]/Commons expects patches and pull requests.

Ta


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Ah, and might be a good idea to run `mvn clean site`, then have a look at 
the reports output. Some pull requests are delayed merging due to the 
introduction of findbugs/checkstyle/etc issues. Quickly running it and fixing 
any reported issues might make it easy and faster to get it merged :)


---


[GitHub] commons-lang issue #311: LANG-1373 Stopwatch based capability for nested, na...

2018-01-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/311
  
Ack :-) haven't had time to review it as we have a short summer around 
here, so have added a note to have a look at StopWatch (which I'm not familiar 
with) and at this variation.

Said that, had a very brief peek at the code from the browser without using 
the IDE. The code looks great! Only small minor things I could see were a 
duplicated white line (which doesn't matter tbh) and the the formatting. If I 
recall correctly, [lang] uses 4 spaces instead of 2? Though I could be wrong.

Thanks for being so patient. I intend to review it as soon as I get some 
spare time (IOW once the weather gets back to our normal 10-17C cloudy days 
with with windy rains). But happy if anyone beats me to it.

Cheers
Bruno 


---


[GitHub] commons-lang issue #312: LANG-1375: defaultString(str) reuses defaultString(...

2018-01-19 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/312
  
Merged. Checked the pull request locally, built the project, ran tests, 
confirmed the diff, and then updated changes.xml.

Thanks again @codingsince1985 !


---


[GitHub] commons-lang issue #309: Fix EventCountCircuitBreaker increment batch

2017-11-26 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/309
  
Thank you for taking your time to submit the pull request, and for closing 
it now :-) 


---


[GitHub] commons-lang issue #309: Fix EventCountCircuitBreaker increment batch

2017-11-25 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/309
  
Merge request merged in 7d061e33e59e23dc4b03378f35f50a7d70f033b3. Alas, I 
forgot to amend the commit to include the alias to close this pull request 
automatically.

@dieb could you close it, please?

Thanks!


---


[GitHub] commons-lang issue #309: Fix EventCountCircuitBreaker increment batch

2017-11-25 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/309
  
Ticket created in JIRA https://issues.apache.org/jira/browse/LANG-1370


---


[GitHub] commons-lang issue #309: Fix EventCountCircuitBreaker increment batch

2017-11-22 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/309
  
I'll leave the ticket open for a while other to be able to review it too. 
And before merging I'll have to create a place holder ticket in JIRA. Or if you 
have time and would like to do that :-) otherwise I'll do as it takes just a 
couple of minutes to create and link to this PR.


---


[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...

2017-11-04 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/275
  
>My suggestion would be to add two new classes with the fixed code 
(different class names), and deprecated the old classes. That way there is no 
need to have commons-lang v4.

Thought a bit about that after seeing the vote for [lang] 3.7. In any way, 
the current implementation will be removed only in 4.x, and we will have to 
include the require static for Java 9 module.

So right now I am thinking about not marking the class or fields as 
deprecated, nor adding another class with a different name. Just keep this PR 
and the ticket open. Then mark the ticket as FixVersion 4.0, use the require 
static trick for the module-info.java for Java 9.

Does that sound like a good plan?


---


[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...

2017-10-23 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/275
  
Yup @britter you are definitely correct. At least if we agree on the 
solution, we can think about marking some methods as deprecated in the current 
circuit breaker (though I prefer to mark as deprecated when there is an 
alternative), and/or add notes in the next release notes, alerting users about 
the change to come.


---


[GitHub] commons-lang issue #299: Add module-info for Java 9

2017-10-22 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/299
  
@michaelsavich I have a proposal in a branch, submitted some months ago 
https://github.com/apache/commons-lang/pull/275

Waiting for feedback to prepare deprecated annotations in place, and merge 
in a major release (I believe it breaks BC)


---


[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...

2017-10-10 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/296#discussion_r143664742
  
--- Diff: src/main/java/org/apache/commons/lang3/time/GmtTimeZone.java ---
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import java.util.Date;
+import java.util.TimeZone;
+
+/**
+ * Custom timezone that contains offset from GMT.
+ *
+ * @since 3.7
+ */
+class GmtTimeZone extends TimeZone {
--- End diff --

TimeZone is Serializabe. Do we need to add a serialVersionUID here?


---


[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...

2017-10-10 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/296#discussion_r143667156
  
--- Diff: src/test/java/org/apache/commons/lang3/time/GmtTimeZoneTest.java 
---
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for GmtTimeZone
+ */
+public class GmtTimeZoneTest {
+
+@Test(expected = IllegalArgumentException.class)
+public void hoursOutOfRange() {
+new GmtTimeZone(false, 24, 0);
+}
+
+@Test
+public void hoursInRange() {
+Assert.assertEquals(23 * 60 * 60 * 1000, new GmtTimeZone(false, 
23, 0).getRawOffset());
+}
+
+@Test(expected = IllegalArgumentException.class)
+public void minutesOutOfRange() {
+Assert.assertEquals(0, new GmtTimeZone(false, 60, 0));
--- End diff --

This test is wrong. Its title states that the minutes will be out of range, 
but the hour is actually out of range (60). Minute is 0, but never gets checked.


---


[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...

2017-10-10 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/296#discussion_r143660909
  
--- Diff: src/main/java/org/apache/commons/lang3/time/FastTimeZone.java ---
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import java.util.TimeZone;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Faster methods to produce custom time zones.
+ *
+ * @since 3.7
+ */
+public class FastTimeZone {
+
+private static final TimeZone GREENWICH = new GmtTimeZone(false, 0, 0);
+
+/**
+ * Get the GMT TimeZone.
+ * @return A TimeZone with a raw offset of zero.
+ */
+public static TimeZone getGmtTimeZone() {
+return GREENWICH;
+}
+
+/**
+ * Get a TimeZone, looking first for GMT custom ids, then falling back 
to Olson ids.
+ * A GMT custom id has an optional prefix of GMT, followed by sign, 
hours digit(s), optional
+ * colon(':'), and optional minutes digits: [GMT] (+|-) Hours [[:] 
Minutes]
+ *
+ * @param id A GMT custom id or Olsen id
+ * @return A timezone
+ */
+public static TimeZone getTimeZone(String id) {
+TimeZone tz = getGmtTimeZone(id);
+if (tz != null) {
+return tz;
+}
+return TimeZone.getTimeZone(id);
+}
+
+private static final Pattern GMT_PATTERN = 
Pattern.compile("^(?:(?i)GMT)?([+-])?(\\d\\d?)?(:?(\\d\\d?))?$");
+
+/**
+ * Get a TimeZone with GMT offsets.  A GMT offset must be either 'Z' 
or match
+ * (GMT)? hh?(:?mm?)?, where h and m are digits representing hours and 
minutes.
--- End diff --

Maybe instead of

> A GMT offset must be either 'Z' or match (GMT)? hh?(:?mm?)?

It should be

> A GMT offset must be 'Z', or 'UTC', or match (GMT)? hh?(:?mm?)?

?




---


[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...

2017-10-10 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/296#discussion_r143663160
  
--- Diff: src/main/java/org/apache/commons/lang3/time/FastTimeZone.java ---
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import java.util.TimeZone;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Faster methods to produce custom time zones.
+ *
+ * @since 3.7
+ */
+public class FastTimeZone {
--- End diff --

Do we need a private constructor to prevent instantiation of FastTimeZone? 
It seems to contain only static methods. Not sure if that's the intended 
design. But noticed it while looking at the cobertura report (which is looking 
great).


---


[GitHub] commons-lang pull request #296: LANG-1355: Add FastTimeZone to decrease Time...

2017-10-10 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/296#discussion_r143667312
  
--- Diff: src/test/java/org/apache/commons/lang3/time/GmtTimeZoneTest.java 
---
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3.time;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for GmtTimeZone
+ */
+public class GmtTimeZoneTest {
+
+@Test(expected = IllegalArgumentException.class)
+public void hoursOutOfRange() {
+new GmtTimeZone(false, 24, 0);
+}
+
+@Test
+public void hoursInRange() {
+Assert.assertEquals(23 * 60 * 60 * 1000, new GmtTimeZone(false, 
23, 0).getRawOffset());
+}
+
+@Test(expected = IllegalArgumentException.class)
+public void minutesOutOfRange() {
+Assert.assertEquals(0, new GmtTimeZone(false, 60, 0));
+}
+
+@Test
+public void minutesInRange() {
+Assert.assertEquals(59 * 60 * 1000, new GmtTimeZone(false, 0, 
59).getRawOffset());
+}
+
+@Test
+public void getOffset() {
+Assert.assertEquals(0, new GmtTimeZone(false, 0, 
0).getOffset(234304));
+}
+
+@Test(expected = UnsupportedOperationException.class)
+public void setRawOffset() {
+new GmtTimeZone(false, 0, 0).setRawOffset(0);
+}
+
+@Test
+public void getRawOffset() {
+Assert.assertEquals(0, new GmtTimeZone(false, 0, 
0).getRawOffset());
+}
+
+@Test
+public void getID() {
+Assert.assertEquals("GMT+00:00", new GmtTimeZone(false, 0, 
0).getID());
+Assert.assertEquals("GMT+01:02", new GmtTimeZone(false, 1, 
2).getID());
+Assert.assertEquals("GMT+11:22", new GmtTimeZone(false, 11, 
22).getID());
+Assert.assertEquals("GMT-01:02", new GmtTimeZone(true, 1, 
2).getID());
+Assert.assertEquals("GMT-11:22", new GmtTimeZone(true, 11, 
22).getID());
+}
+
+@Test
+public void useDaylightTime() {
+Assert.assertFalse(new GmtTimeZone(false, 0, 0).useDaylightTime());
+}
+
+@Test
+public void inDaylightTime() {
+Assert.assertFalse(new GmtTimeZone(false, 0, 0).useDaylightTime());
+}
--- End diff --

Maybe add something like

```
@Test
public void testToString() {
Assert.assertEquals("[GmtTimeZone 
id=\"GMT+23:00\",offset=8280]", new GmtTimeZone(false, 23, 0).toString());
}

@Test
public void testGetOffset() {
Assert.assertEquals(8280, new GmtTimeZone(false, 23, 
0).getOffset(1, 1, 1, 1, 1, 1));
}
```

With these two tests we reach 100% for GmtTimeZone.


---


[GitHub] commons-lang issue #298: Add an instanceof test in the implementation of equ...

2017-10-09 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/298
  
I think it makes sense. Any chance to include a unit test to protect 
against regressions in the future? Thanks


---


[GitHub] commons-lang issue #279: Fraction debug printings

2017-09-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/279
  
I think what Gary meant is that that comment is harmless, and can be used 
by other developers that want/need to debug the code, and see what is going on. 
I've used it in some projects myself, to indicate to fellow developers parts of 
the code that may be a bit fuzzy or complicated. I'm OK if it's kept, but OK 
too if others prefer to remove it (if we replace it by a comment like "Here we 
can see variable x" I'm OK as well).


---


[GitHub] commons-lang issue #288: Fix typo stateme->statement

2017-09-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/288
  
Merged. Thanks!!!


---


[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...

2017-09-09 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/285
  
Thank you for updating the pull request.

Tested locally. Using the changes in the pull request I get the same result 
as we get when the settings are in the Maven settings.

Commenting out the suppression filter in the checkstyle configuration, the 
report in the web site now gives me **2828** errors. So the suppression filter 
is working indeed.

Looks good to me, merging it now.


---


[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...

2017-09-08 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/285
  
>Does it really generates a site for the current branch?

It should. That is one step in the release process. Probably due to the 
site using another configuration (under the reporting tag).


https://github.com/osiegmar/commons-lang/blob/01a820275fc7bd66cb699dd081a432c7dde2645e/pom.xml#L711

Could you update the pull request with your original change duplicated in 
that portion, please?


---


[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...

2017-09-08 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/285
  
Not sure. Here's what I just tried.

```
$ git reset --hard
$ git status 
On branch pr/285

$ vim checkstyle.xml
$ # removed  line
$ mvn clean site
```

And then opening the Checkstyle report (via 
file:///home/.../commons-lang/target/site/index.html) there are no warnings or 
error in the checkstyle page.

Are you able to spot which step I am missing to reproduce it?


---


[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...

2017-09-08 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/285
  
Checked out pull request branch, built with the change, got no checkstyle 
errors. Then removed the suppression (without re-adding the previous value in 
pom.xml) and also got the same result, without any errors. Maybe it is aware 
that the suppression file must be ignored?

+1 for the change. Will wait for others to review :+1: 

Documentation about the SuppressionFilter module.

http://checkstyle.sourceforge.net/config_filters.html#SuppressionFilter

Thank you for your contribution @osiegmar 


---


[GitHub] commons-lang issue #282: SwappedPair constructed as Pair.of(rhs,lhs)

2017-08-11 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/282
  
Makes sense, good catch. However, would be nice to have a unit test at 
least to cover that case, and make sure we don't have any regression for this 
behaviour. Would you be able to create one @namannigam ?

Thanks!!!
Bruno


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #275: [WIP] LANG-1339: replace java.beans.Property...

2017-07-09 Thread kinow
GitHub user kinow opened a pull request:

https://github.com/apache/commons-lang/pull/275

[WIP] LANG-1339: replace java.beans.PropertyListener by java.util.Observable

For discussion, regarding 
[LANG-1339](https://issues.apache.org/jira/browse/LANG-1339) (TL;DR: remove 
depedency on java.desktop from [lang], preparing for Java 9). Thought having 
some code to discuss would be helpful.

This pull request contains a possible solution, where 
java.beans.{PropertyChangeListener,PropertyChangeListener} are replaced by 
java.util.{Observer,Observable}.

Users would still have the same feature, but alas binary compatibility 
would not be kept. So perhaps this could be in a 4.x release. The other methods 
are deprecated in the next 3.x releases, and we remove the dependency to 
java.desktop on 4.x, and also offer a Java9 module with no dependency 
java.desktop.

Would this be a viable alternative?

Cheers
Bruno

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kinow/commons-lang LANG-1339

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-lang/pull/275.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #275


commit 0ba4ea983b0e2ffdffd102b23921292c3c92d3ac
Author: Bruno P. Kinoshita <ki...@apache.org>
Date:   2017-07-09T10:01:06Z

LANG-1339: replace java.beans.PropertyListener by java.util.Observable




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #268: .travis.yml: add oraclejdk9

2017-06-12 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/268
  
Ooh, looked at the wrong green icon. JDK 7 complained about JAVA_HOME 
location... weird. Related issue in travis-ci repo 
https://github.com/travis-ci/travis-ci/issues/5520


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #268: .travis.yml: add oraclejdk9

2017-06-12 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/268
  
+1 lgtm!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #269: LANG-1337: Fix test failures in IBM JDK 8 for ToStr...

2017-06-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/269
  
Comments added, received some feedback, but would still be useful someone 
with the last IBM JDK 8 to give it a try and confirm it works for him/her :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-07 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/269#discussion_r120550160
  
--- Diff: 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java ---
@@ -316,7 +317,7 @@ public void testReflectionHierarchyArrayList() {
 // representation different for IBM JDK 1.6.0, LANG-727
 assumeFalse("IBM Corporation".equals(SystemUtils.JAVA_VENDOR) && 
"1.6".equals(SystemUtils.JAVA_SPECIFICATION_VERSION));
 assumeFalse("Oracle Corporation".equals(SystemUtils.JAVA_VENDOR) 
&& "1.6".compareTo(SystemUtils.JAVA_SPECIFICATION_VERSION) < 0);
-final List list = new ArrayList<>();
+final List list = new 
ArrayList<>(arraylistInitialCapacity);
--- End diff --

Roger that. Will add a note to myself to fix the other final member 
variables later... trying to be concise, but I'm clearly missing the point here 
:-) was supposedly to be a very simple fix for this issue. Pushing a new commit 
in a few minutes, just finishing to review commons-fileupload vote.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-07 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/269#discussion_r120546260
  
--- Diff: 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java ---
@@ -316,7 +316,7 @@ public void testReflectionHierarchyArrayList() {
 // representation different for IBM JDK 1.6.0, LANG-727
 assumeFalse("IBM Corporation".equals(SystemUtils.JAVA_VENDOR) && 
"1.6".equals(SystemUtils.JAVA_SPECIFICATION_VERSION));
 assumeFalse("Oracle Corporation".equals(SystemUtils.JAVA_VENDOR) 
&& "1.6".compareTo(SystemUtils.JAVA_SPECIFICATION_VERSION) < 0);
-final List list = new ArrayList<>();
+final List list = new ArrayList<>(10);
--- End diff --

>If the test fails when the initial size arg is omitted, does that not also 
affect the behaviour of the method being tested?

Not really. The test simply checks the string built for an arraylist 
through reflection. The issue was caused for believing that the lazy 
initialization (as @andyklimczak) would work in the same independent of the JVM.

What the test is verifying is correct, the current approach could be 
improved to make the test less flaky.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-07 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/269#discussion_r120545692
  
--- Diff: 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java ---
@@ -316,7 +316,7 @@ public void testReflectionHierarchyArrayList() {
 // representation different for IBM JDK 1.6.0, LANG-727
 assumeFalse("IBM Corporation".equals(SystemUtils.JAVA_VENDOR) && 
"1.6".equals(SystemUtils.JAVA_SPECIFICATION_VERSION));
 assumeFalse("Oracle Corporation".equals(SystemUtils.JAVA_VENDOR) 
&& "1.6".compareTo(SystemUtils.JAVA_SPECIFICATION_VERSION) < 0);
-final List list = new ArrayList<>();
+final List list = new ArrayList<>(10);
--- End diff --

Fair enough on the magic number. I'd thought about that, then noticed a few 
other tests with numbers. But one broken window doesn't mean I can break 
another one :-) fixing in another commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-06 Thread kinow
GitHub user kinow opened a pull request:

https://github.com/apache/commons-lang/pull/269

LANG-1337: Fix test failures in IBM JDK 8 for ToStringBuilderTest by 
specifying the ArrayList initial capacity.

See https://issues.apache.org/jira/browse/LANG-1337 for issue description 
and explanation of the changes here.

In summary, we specify the ArrayList's initial capacity, this way the 
String built through reflection has the expected value. Tested with Oracle JDK 
7 and 8, and IBM JDK 8.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kinow/commons-lang LANG-1337

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-lang/pull/269.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #269


commit 65b08c4f91a4e5d78eb645b46d06d7a46f8c62dd
Author: Bruno P. Kinoshita <ki...@apache.org>
Date:   2017-06-06T11:41:31Z

LANG-1337: Fix test failures in IBM JDK 8 for ToStringBuilderTest by 
specifying the ArrayList initial capacity.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

2017-05-01 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/209
  
Oh, TIL. I think we can't close this pull request, and wait in case a user 
needs this feature :+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #209: LANG-1164: allow ToStringStyle to omitNulls

2017-05-01 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/209
  
+1 lgtm

Looks useful, I've wanted that feature a couple of times when using 
ReflectionToStringBuilder in the past.

Merging the code locally, the build passes, all reports look good but 
Chekstyle.

```
org/apache/commons/lang3/builder/ToStringStyle.java
SeverityCategoryRuleMessage Line
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  474
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  908
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1005
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1067
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1129
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1191
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1253
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1315
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1377
 Error  blocks  NeedBraces  'if' construct must use '{}'s.  1439
```

The inlined if fails with our current checkstyle rules. Other than that, 
all looks good :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #209: LANG-1164: allow ToStringStyle to omitNulls

2017-05-01 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/209#discussion_r114114198
  
--- Diff: src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java 
---
@@ -467,6 +472,7 @@ protected void removeLastFieldSeparator(final 
StringBuffer buffer) {
  *  for summary info, null for style decides
  */
 public void append(final StringBuffer buffer, final String fieldName, 
final Object value, final Boolean fullDetail) {
+if (omitNulls && value == null) return;
--- End diff --

With our current checkstyle checks, this will create an error (error in 
Checkstyle, not a build error).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #266: docs - fixed faulty samples of isNoneEmpty/isNoneBl...

2017-05-01 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/266
  
Docs look good, reviewed code locally, examples in the JavaDocs look more 
homogeneous, always displaying the behaviour for arrays with no elements, and 
for arrays with just a blank string.

Merged. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #265: LANG-1325: Increase test coverage of ToStringBuilde...

2017-04-29 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/265
  
That looks good, thanks! The coveralls bot normally tells us here if the 
coverage increased or decreased. However, looks like you have added tabs to 
your code, and that caused Travis CI build bot to fail (click on Details on the 
link above for details):

```
#https://travis-ci.org/apache/commons-lang/jobs/227134274

INFO] There is 1 error reported by Checkstyle 6.11.2 with 
/home/travis/build/apache/commons-lang/checkstyle.xml ruleset.

[ERROR] 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java:[617,1] 
(whitespace) FileTabCharacter: File contains tab characters (this is the first 
instance).

[INFO] 


[INFO] BUILD FAILURE

[INFO] 


[INFO] Total time: 01:00 min

[INFO] Finished at: 2017-04-29T13:20:38+00:00

[INFO] Final Memory: 39M/431M

[INFO] 


[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check (default-cli) on 
project commons-lang3: You have 1 Checkstyle violation. -> [Help 1]
```

If you remove tabs by spaces it should pass the build and we will see the 
coverage report here.

Thanks!
Bruno


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #:

2017-04-18 Thread kinow
Github user kinow commented on the pull request:


https://github.com/apache/commons-lang/commit/dfecbe970917754511a081f8b86efac211e624f6#commitcomment-21810221
  
+1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #257: Apply checkstyle to test sources

2017-03-14 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/257
  
Checked pull request locally, build passes OK. The master branch checkstyle 
report has 0 errors, in 154 files. This PR branch checkstyle report has 0 
errors, in 331 files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #257: Apply checkstyle to test sources

2017-03-14 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/257
  
Not sure about applying that to test sources. It's normal to apply it to 
sources, but I must confess I'm less strict about issues in the tests. Normally 
quite happy about having the tests. As long as the tests are clear and easy to 
understand, I'm happy to manually fix a few tabs, add some extra classes or 
annotations, instead of block a commit to ask a contributor to fix the tests.

+0


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #254: Add checkstyle to the CI

2017-03-11 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/254#discussion_r105530594
  
--- Diff: checkstyle.xml ---
@@ -36,11 +36,9 @@ limitations under the License.
 
 
 
-
--- End diff --

:+1: thanks for the explanation and the links @mureinik 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #254: Add checkstyle to the CI

2017-03-11 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/254#discussion_r105530165
  
--- Diff: 
src/main/java/org/apache/commons/lang3/concurrent/annotation/package-info.java 
---
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/**
+ * Provides annotations to document classes' thraed safety
--- End diff --

typo

s/thraed/thread


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #254: Add checkstyle to the CI

2017-03-11 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/254#discussion_r105530182
  
--- Diff: checkstyle.xml ---
@@ -36,11 +36,9 @@ limitations under the License.
 
 
 
-
--- End diff --

Any reason for removing this one? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108037
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105107714
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108083
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108009
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
--- End diff --



Minor bug in Javadoc HTML. Extra opening paragraph. Can be fixed later too.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108108
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108206
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108158
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108139
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108059
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108338
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105106940
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
--- End diff --

Author classes are not used any more in Commons. We have removed most, and 
now we maintain the list of contributors in the source control tool + pom.xml.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105108179
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
+
+// x86 mappings
+addX86(X86);
+addX86("i386");
+addX86("i486");
+addX86("i586");
+addX86("i686");
+addX86("pentium");
+
+// x86_64 mappings
+addX86_64(X86_64);
+addX86_64("amd64");
+addX86_64("em64t");
+addX86_64("universal"); // Needed for openjdk7 in Mac
+
+// Itenium 64-bit mappings
+addIA64(IA64);
+addIA64("ia64w");
+
+// Itenium 32-bit mappings, usually an HP-UX construct
+addIA64_32(IA64_32);
+addIA64_32("ia64n");
+
+// PowerPC mappings
+addPPC(PPC);
+addPPC("power");
+addPPC("powerpc");
+addPPC("power_pc");
+addPPC("power_rs");
+
+// PowerPC 64bit mappings
+addPPC64(PPC64);
+addPPC64("power64");
+addPPC64("powerpc64");
+addPPC64("power_pc64");
+addPPC64("power_rs64");
+}
+
+/**
+ * Possibility to add {@link String}, representing the 
System.getProperty("os.arch")
+ * to x86 architecture.
+ *
+ * @param value The {@link String} to add.
+ */
+public static final void addX86(String value) {
+map.put(value, X86);
+}
+
+/**
+ * Checks if the current running JVM is a JVM for x86 architecture.
+ * It returns true, if the os.arch System Property 
matches the following {@link String}'s:
+ * 
+ * x86
+ * i386
+ * i486
+ * i586
+ * i686
+ * pentium
+ * 
+ * 
+ * It is possible to extend the {@link String}'s above using method 
{@link ArchUtils#addX86(String)}.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture 
used by the JVM
+ * not of the operating system.
+ * 
+ *
+ * 

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105107852
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
--- End diff --

Minor bug in Javadoc HTML. Extra opening paragraph. Can be fixed later too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105107484
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new ConcurrentHashMap<>();
--- End diff --

Much better, from a concurrency point of view :-)

Now my preference would be do not allow the user to change it. Otherwise 
you could accidentally remove one arch, or overwrite it. Besides that, I don't 
see why we need to let users change that.

If there is an architecture that is not supported, I think it would be 
better that users will apply a temporary patch to their code and/or report an 
issue. Then we will update the list of architectures.

Maybe we could include it as in [crypto] @Tomschi , as a read-only 
structure for now, and then later discuss about exposing methods to manipulate 
the map. What do you think? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-07 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r104819886
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new HashMap<>();
+
--- End diff --

Indeed. The crypto implementation doesn't allow changes to the map after it 
has been created. I think we can use a ConcurrentHashMap, or simply follow what 
was done in crypto.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
Excellent @Tomschi 

I'm dropping a note to the mailing list to ask for feedback from crypto 
devs, as there could be some synergy.

I will play with the code at home, but one thing that I noticed is that it 
seems to contain tabs... minor nit pick, but could you check checkstyle and 
make sure the code is not introducing any warnings / errors there, please? That 
way it will be easier to merge the PR.

Thanks!
B


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
Looks better now @Tomschi did you use Crypto's code as reference? cc 
@michael-o 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #247: JUnit imports

2017-02-28 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/247
  
:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #246: DateUtilsTest asserts

2017-02-27 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/246
  
:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

2017-02-22 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
@C0rWin what about trying to add the test from 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110) to the pull 
request? Look at the attachments section.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

2017-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Tried running the following to test the pull request:

```
$ mvn clean install
$ mvn test -Pbenchmark -e -X
```

And it kept failing with:

```
[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 9.718 s
[INFO] Finished at: 2017-02-22T20:00:57+13:00
[INFO] Final Memory: 32M/486M
[INFO] 

[ERROR] Failed to execute goal 
org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project 
commons-lang3: Command execution failed. Process exited with an error: 1 (Exit 
value: 1) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute 
goal org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project 
commons-lang3: Command execution failed.
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
at 
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
at 
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
at 
org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
at 
org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoExecutionException: Command 
execution failed.
at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:302)
at 
org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
... 20 more
Caused by: org.apache.commons.exec.ExecuteException: Process exited with an 
error: 1 (Exit value: 1)
at 
org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
at 
org.apache.commons.exec.DefaultExecutor.execute(DefaultExecutor.java:166)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:764)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:711)
at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:289)
... 22 more
```

Tried playing with dependency versions and settings, using Commons CSV as 
example (which works for me, as long as manually install one maven dependency 
as per pom.xml comment) with no luck.

Will give it another try tomorrow at work.

```
$ mvn -v
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 
2015-11-11T05:41:47+13:00)
Maven home: /opt/maven
Java version: 1.8.0_121, vendor: Oracle Corporation
Java home: /usr/lib/jvm/java-8-oracle/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.4.0-62-generic", arch: "amd64", family: "unix"
```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

2017-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Oh, missed that one, will take a look today to compare with [csv].

I've never seen it failing in Jenkins or Travis CI, but I suspect it is run 
manually, whenever someone needs to work on the classes involved in the 
benchmark tests for new features or debugging issues in different environments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

2017-02-21 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
I was waiting for @C0rWin to update the pull request, to have an approach 
similar to what we have in other commons projects. If he has updated it, then 
+1 to merging (or maybe check with Emmanuel Bourg if it looks good too?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #239: LANG-1312: LocaleUtils#toLocale does not support la...

2017-02-20 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/239
  
Will merge it tomorrow if there's no objection (or feel free to merge it 
@PascalSchumacher  :-) )


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-02-20 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
@michael-o 

>What is the real pupose for this actually? The client should not care 
about the arch at all.

I think @Tomschi use case is valid, where a client could need to know the 
arch before loading a certain library, and we had another issue submitted 
LANG-1145 with similar requirement.

>The regex match is brittle. This will likely fail on ARM and it fails here 
on Itanium with HP-UX for os.arch IA64N which is a 32 bit VM.

Note taken, perhaps before merging we can try to cover more archs, like 
this list:

* http://lopica.sourceforge.net/os.html

There is another place where arch is used within Commons too:

* 
https://github.com/apache/commons-crypto/blob/158be0644c353a617427ab190a4f09082cda42ac/src/main/java/org/apache/commons/crypto/OsInfo.java#L28

We can possibly look at how crypto is using it, and if we could maybe use a 
similar approach here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


  1   2   >