[GitHub] commons-lang issue #238: Validate's String.format without arguments

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

https://github.com/apache/commons-lang/pull/238
  
@mureinik I think removing these calls should be fine. Specially given that 
other classes in lang use a normal String, and not `String.format` when there 
are no parameters, e.g. 
https://github.com/apache/commons-lang/blob/954ade4c1ae2adc0aaac3a1dbe800495c519520c/src/main/java/org/apache/commons/lang3/ThreadUtils.java#L54


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


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

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

https://github.com/apache/commons-lang/pull/239
  
Yesterday learned something new about `String.format` via pull request 
#238, and now learning about [UN M.49](https://en.wikipedia.org/wiki/UN_M.49). 
Thanks again @PascalSchumacher :-)

Code and test look good, checked out locally, all tests passing. +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 #238: Validate's String.format without arguments

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

https://github.com/apache/commons-lang/pull/238
  
Didn't know about this use of `String.format` @PascalSchumacher , thanks :-)

I'm +1 for merging the pull request. Agree it's unlikely, and not sure if 
its use was intentional.


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


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

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

https://github.com/apache/commons-lang/pull/231
  
Oh, good point @PascalSchumacher have no objection to it. We could probably 
avoid a few misunderstandings that way. Happy with that too @Tomschi ?


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


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

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

https://github.com/apache/commons-lang/pull/231
  
Gotcha, found this example 
https://github.com/Tomschi/jacob-parent/blob/ec3f1c10169c26f14ee1f61bd6622c67a73e26fc/jacob/src/main/java/com/jacob/com/LibraryLoader.java#L202

Looks like a valid use case. I'm all right with merging it, my +0. Let's 
now wait to see what others think about it.

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 #231: Evaluate Architecure

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

https://github.com/apache/commons-lang/pull/231
  
Thanks for updating the pull request @Tomschi.

I don't have a use case for this. I can see where it could be used, but I 
don't have any project where I would use it. Code is clear, javadocs have been 
updated :-)

So will leave it here for others to review, comment, and maybe perhaps.


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


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

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

https://github.com/apache/commons-lang/pull/231
  
I remember a discussion about it some time ago. 

The issue with this approach was that os.arch tells only the JVM arch, not 
really OS arch. 

If there is a strong use case for these properties, we would have to be 
careful with the javadocs. Right now the javadocs don't explicitly tell the 
user that it is the JVM arch, and not the OS arch.

Other approaches involve reflection or JNI, to find out the OS arch, but I 
think that is a bit too tricky to get it done correctly.


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


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

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

https://github.com/apache/commons-lang/pull/182
  
Hi @C0rWin 

Some time ago I found this pull request and had to learn what was JMH :-) 
added a comment on 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), and today had 
time to revisit the issue and take a look at the replies.

So [Commons 
CSV](https://github.com/apache/commons-csv/blob/a7757849d3e9b2a37967ce208ab22f6d14567c98/pom.xml#L389)
 and [Commons 
RNG](https://github.com/apache/commons-rng/tree/688f3611131e05a072d998edb93e13492c3ed256/commons-rng-jmh)
 use JMH too. Both projects apply a similar approach, with the main difference 
being that Commons RNG - maybe for being a multi-module project - contains a 
jmh Maven module.

However, the dependencies and plug-in execution are part of a benchmark 
profile. Following the suggestions in 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), would be nice if 
we:

* Used a similar approach, such as profile, class names, etc
* Update [jmh dependencies to the 
latest](http://search.maven.org/#search%7Cga%7C1%7Cjmh-core)

I wanted to check with you before I opened a separate pull request. Would 
you be willing to update this pull request again with these changes? If so, 
just ping me and I'll reply here as soon as possible. Have plenty of time to 
work on Open Source projects in the next two months :-)

Cheers
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 issue #227: Improve test covarage

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

https://github.com/apache/commons-lang/pull/227
  
Merged. Thanks @Abrasha !


---
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 #:

2016-10-31 Thread kinow
Github user kinow commented on the pull request:


https://github.com/apache/commons-lang/commit/103b64a373256feae6ca85f2bf220e7694e48fa4#commitcomment-19637078
  
:grinning: 


---
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 #198: LANG-1269: Wrong name or result of StringUtils#getJ...

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

https://github.com/apache/commons-lang/pull/198
  
+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 #199: LANG-1258: Add ArrayUtils#toStringArray(Object[]) m...

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

https://github.com/apache/commons-lang/pull/199
  
Patch looks good. I wonder if the inline if will raise a warning in 
checkstyle. Other than that, +1 :D


---
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 #195: LANG-1160 StringUtils.abbreviate() to suppor...

2016-10-13 Thread kinow
GitHub user kinow opened a pull request:

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

LANG-1160 StringUtils.abbreviate() to support 'custom ellipses' parameter

From [LANG-1160](https://issues.apache.org/jira/browse/LANG-1160).

Before this pull request the code contained only two abbreviate methods. 
One abbreviate(String, int) that would call the other one abbreviate(String, 
int, int) with a default value for one of the int parameters.

This pull request adds two other methods with the same name, but that 
receive a custom abbreviation marker. The existing methods were update to call 
the new complete method with a default abbreviation marker of "...", which 
guarantees backward compatibility (we are keeping binary compatibility, and 
also feature backward compatibility).

Tests were update, test coverage is the same as before. No issues found 
building the site locally and from looking at CPD, PMD, FindBugs, CheckStyle 
and Surefire reports; all tests passing locally, with additional two tests in 
this PR.

Cheers
Bruno

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

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

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

https://github.com/apache/commons-lang/pull/195.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 #195


commit a7b552208acb7d4eb6d79bddb60cb358ae1ce5bd
Author: Bruno P. Kinoshita <ki...@apache.org>
Date:   2016-10-13T09:55:17Z

LANG-1160 StringUtils.abbreviate() to support 'custom ellipses' parameter




---
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 #194: add isAllBlank,isNotAllBlank method for String "nul...

2016-10-12 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/194
  
Likewise @wangdongxun :-) are you albe to close this pull request? 
Otherwise I believe there is some integration in our infrastructure to let us 
close it.

Please do not hesitate to submit other pull requests, issues or comment in 
our mailing lists. 

Thank you
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 issue #189: new impl of LevenshteinDistance

2016-10-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/189
  
Ack @PascalSchumacher after @yufcuy 's feedback we can merge it and include 
in 3.x releases, and then think where, and if, we should move text-related code 
:-)

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 #194: add isAllBlank,isNotAllBlank method for String "nul...

2016-10-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/194
  
Thanks for your contribution @wangdongxun !

>The method would be very confusing, as the String "null" is not all bank.

Agree with @PascalSchumacher . Adding this could indeed be very confusing. 
Future requests could ask for "'null'" (with single quotes inside), or "NULL", 
etc. Easier done with two calls as @PascalSchumacher suggested IMO.

Unfortunately -1 as well.


---
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 #189: new impl of LevenshteinDistance

2016-10-07 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/189
  
Hi @yufcuy,

Sorry for the delay to look into this.

I looked at the first two implements this morning to refresh my memory. The 
first one creating the whole comparison table, and the second one with just the 
current and previous row. Both described in the Wikipedia page you linked 
(thanks for that).

Then I started reviewing your pull request, and I believe your 
implementation is correct :-) though I couldn't find the exact algorithm 
implementation description on Wikipedia. The best I could find was this page: 
http://blog.softwx.net/2014/12/optimizing-levenshtein-algorithm-in-c.html

The page mentioned above mentions the single-array approach. We could add a 
link to it in the Javadoc, as for the previous two implementations. What do you 
think?

There are other trivial changes regarding spelling, typos, tabs vs. spaces, 
etc. So I will add a few more comments, but all tests are passing, I see no 
regression (feature-wise, or in performance), and if you agree with the minor 
adjustments we may have to do, then I believe we are ready to merge it.

@britter should we keep it in [lang], or in [text]? Either way, I will 
replicate the change in the Levenshtein implementation in [text] :-)

Cheers
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 issue #189: new impl of LevenshteinDistance

2016-09-14 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/189
  
Though I also agree with @britter that some better description would help 
reviewing it :-)


---
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 #189: new impl of LevenshteinDistance

2016-09-14 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/189
  
I think string distances were the one of the first things we thought about 
moving to [text]. Might be interesting to discuss implementing - if it makes 
sense - this change there.


---
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 #171: Removing test redundant for org.apache.commons.lang...

2016-07-15 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/171
  
@august782, thanks for being understanding. I think finding redundant 
tests, and simplifying it is a good idea, just not in the case of tests 
covering many different scenarios.

I think many tests in Apache Commons components are written with scenarios 
in mind, and method-oriented. As normally the components are libraries to be 
included in other systems, the tests aim at asserting that methods work as 
expected, under different conditions (i.e. with a diverse range of possible 
arguments).

Having good code coverage is important as well, but tests are not being 
written in Apache Commons components simply to have a high score in the reports 
- though normally covering well your scenarios, you end up with a good branch 
and line test coverage as well (citation needed :) ).

So if you have found redundant tests, where the same scenario is being 
covered in multiple tests, and you believe that reducing it will i) benefit 
users and developers in understanding the code, and ii) still cover the 
possible use cases for the library, or iii) make it easier to assert the same 
thing, then I think these pull requests would be useful.

For other pull requests that disable or remove tests based only on line and 
or branch coverage, I would still argue against removing them, and in favor to 
keep them; for the reasons I stated before.


---
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 #171: Removing test redundant for org.apache.commons.lang...

2016-07-15 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/171
  
Both tests use ClassUtils#wrapperToPrimitive. One passes a String.class, 
and the other a null value. Then the method looks at a map, created and filled 
in a static constructor. 

Since String.class is not in the map, it returns a null value.

Then trying to get a value for the key null, also returns a null value.

I agree that, considering code coverage, both tests are redundant. But they 
are assessing two different things, as defined in the method names. One 
assessing that asking for null will return a null. And the other that 
requesting an element that is not in the map also returns null.

Even though they may be redundant now, something may change in the future, 
and having tests covering scenarios, regardless of coverage, is a good thing 
too.

>If you do not believe any of these tests should be ignored, we would 
greatly appreciate it if you could follow up on this pull request and let us 
know your reasons.

So for this, I'm -1 for removing the test based only on coverage. They may 
be redundant, looking from a test coverage perspective, but as a developer 
using that method, I expect to be able to quickly understand what's going to 
happen in each scenario I use it. Tests and Javadoc with some examples, 
covering these scenarios, can be extremely useful.


---
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: Fix for incorrect comment on StringUtil...

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/125#issuecomment-213862327
  
Adding an entry to changes.xml with dev=ggregory and the text "... closes 
#125" in order to close this pull request.


---
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: Fix for incorrect comment on StringUtil...

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/125#issuecomment-213862318
  
Placeholder ticket created https://issues.apache.org/jira/browse/LANG-1222


---
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: Fix typo on appendIfMissing javadoc

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/129#issuecomment-213861995
  
Adding an entry in changes.xml for that with dev= ggregory, with a message 
containing the closes #129 in order to close this pull request in GitHub and 
also keep track of the change.


---
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: Fix typo on appendIfMissing javadoc

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/129#issuecomment-213861965
  
Placeholder ticket created https://issues.apache.org/jira/browse/LANG-1221


---
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: Add tests for missed branches in DateUt...

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/133#issuecomment-213860528
  
Tested locally. All tests pass. Line coverage for Date keeps in 95%, branch 
coverage went from 90% to 92%, having 194/210 whereas it previously was 189/210.

Couldn't merge with the normal commands locally, as the remote branch was 
called 'master' and git wasn't happy about that. Didn't have enough git-fu for 
a workaround, so merged manully as a patch (see the bottom of the page at 
http://wiki.apache.org/commons/UsingGIT).

Thanks for your contribution.
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: Add tests for missed branches in DateUt...

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/133#issuecomment-213860127
  
Merged in 
https://github.com/apache/commons-lang/commit/fac65b868c549d28b786568636bbc8362d2a78f2


---
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: Add tests for missed branches in DateUt...

2016-04-23 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/133#issuecomment-213859206
  
Placeholder ticket created in JIRA 
https://issues.apache.org/jira/browse/LANG-1220


---
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: random length strings within range

2015-06-28 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/101#issuecomment-116237900
  
#94, #100 and #101 are useful for generating random data for (for example) 
testing. I thought about suggesting these methods to the incubating [text] 
component; but on second thought, I think it could be better to have these 
features in a separate component.

Just food for thought, but maybe something like a commons for testing, like 
https://github.com/andygibson/datafactory, but also including general code for 
helping Java developers to write test.


---
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: Ability to throw checked exceptions wit...

2015-06-28 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/98#issuecomment-116250029
  
+1 @jochenw. Do you think it would be a good idea to start a discussion 
thread in the dev mailing list as well?


---
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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/98#issuecomment-113741510
  
@gzak feel free to update the pull request  comment here or in JIRA, 
otherwise we may close it as the current implementation needs rework.

Nice catch @netomi @jochenw ! 


---
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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/98#issuecomment-113734542
  
Place holder ticket created in JIRA 
https://issues.apache.org/jira/browse/LANG-1149

Feel free to write to the Commons Dev Mailing list (use the [lang] prefix) 
about your issue if this pull request stays idle for too long or if you want to 
discuss the implementation. Thank you for your contribution.


---
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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on the pull request:

https://github.com/apache/commons-lang/pull/98#issuecomment-113734438
  
Checked out the pull request locally, it compiles and the tests pass. The 
minor checkstyle errors, the author tag and the missing license headers have to 
be fixed; you can either update the pull request or a committer will do so when 
the code is merged.

As @sjamesr commented it looks like Throwables#propagate from Guava. I'm +1 
for the feature, though I'm not sure about the class name. Will leave it open 
for a while so that others can review it 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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/98#discussion_r32883472
  
--- Diff: src/main/java/org/apache/commons/lang3/exception/ThrowUtils.java 
---
@@ -0,0 +1,35 @@
+package org.apache.commons.lang3.exception;
--- End diff --

Missing license header.


---
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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/98#discussion_r32883469
  
--- Diff: 
src/test/java/org/apache/commons/lang3/exception/ThrowUtilsTest.java ---
@@ -0,0 +1,30 @@
+package org.apache.commons.lang3.exception;
--- End diff --

Missing license header.


---
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: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/98#discussion_r32883445
  
--- Diff: src/main/java/org/apache/commons/lang3/exception/ThrowUtils.java 
---
@@ -0,0 +1,35 @@
+package org.apache.commons.lang3.exception;
+
+/**
+ * @author a href=mailto:theg...@gmail.com;Gregory Zak/a
+ *
+ * pUtility class containing a method to allow checked exceptions to be 
rethrown without explicitly
+ * declaring them./p
+ *
+ */
+public final class ThrowUtils {
+
+private ThrowUtils() {
+}
+
+/**
+ * pAny method with uninteresting checked exceptions in the body can 
simply be wrapped in a
+ * try/catch block, and the exceptions can then be freely rethrown 
from the catch block using
+ * {@code rethrow(t)}. If a method requires a return value, simply use 
{@code return rethrow(t)}
+ * in the catch block and compiler type inference will do the rest (no 
need to manually capture
+ * a return variable in the try block and return it after the catch 
block)./p
+ *
+ * @param throwable The throwable to rethrow
+ * @param R Used only for compiler type inference
+ * @return Never returns, always throws
+ */
+public static R R rethrow(Throwable throwable) {
+return ThrowUtils.R, RuntimeExceptionrethrow0(throwable);
+}
+
+@SuppressWarnings(unchecked)
+private static R, T extends Throwable R rethrow0(Throwable 
throwable) throws T {
--- End diff --

A JavaDoc comment is necessary or it will add a CheckStyle 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 pull request: Ability to throw checked exceptions wit...

2015-06-20 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/98#discussion_r32883446
  
--- Diff: src/main/java/org/apache/commons/lang3/exception/ThrowUtils.java 
---
@@ -0,0 +1,35 @@
+package org.apache.commons.lang3.exception;
+
+/**
+ * @author a href=mailto:theg...@gmail.com;Gregory Zak/a
+ *
+ * pUtility class containing a method to allow checked exceptions to be 
rethrown without explicitly
+ * declaring them./p
+ *
+ */
+public final class ThrowUtils {
+
+private ThrowUtils() {
--- End diff --

A JavaDoc comment is necessary or it will add a CheckStyle 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 pull request: Ability to throw checked exceptions wit...

2015-06-19 Thread kinow
Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/98#discussion_r32881012
  
--- Diff: src/main/java/org/apache/commons/lang3/exception/ThrowUtils.java 
---
@@ -0,0 +1,35 @@
+package org.apache.commons.lang3.exception;
+
+/**
+ * @author a href=mailto:theg...@gmail.com;Gregory Zak/a
--- End diff --

@author tag needs to be removed (project policy)


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


<    1   2