[GitHub] commons-lang issue #391: Adding junits for JsonToStringStyle
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
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...
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 ...
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...
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...
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...
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...
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
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 ...
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 ...
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 ...
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...
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") hÌ >>> print(u"\u0067\u0307") gÌ >>> 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...
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 ...
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
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
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
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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(...
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
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
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
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
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...
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...
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
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...
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...
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...
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...
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...
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...
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
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
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...
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...
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...
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...
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)
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...
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
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
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...
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...
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...
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...
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...
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
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
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
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...
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...
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 #:
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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.
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...
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
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. ---