You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by TheRealHaui <gi...@git.apache.org> on 2017/09/20 18:08:59 UTC
[GitHub] commons-text pull request #63: small-code-quality-improvements
GitHub user TheRealHaui opened a pull request:
https://github.com/apache/commons-text/pull/63
small-code-quality-improvements
Made small code quality improvements.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/TheRealHaui/commons-text small-code-quality-improvements
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-text/pull/63.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 #63
----
commit d5c840c19792b6c0dee03e0779ac791c73f296cf
Author: Michael Hausegger <ha...@googlemail.com>
Date: 2017-09-20T18:03:24Z
small-code-quality-improvements Made small code quality improvements.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by chtompki <gi...@git.apache.org>.
Github user chtompki commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140545049
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,10 +614,7 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
- newCodePoint = Character.toLowerCase(oldCodepoint);
- whitespace = false;
- } else if (Character.isTitleCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
--- End diff --
Checkstyle want's to see this as
```java
if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint)) {
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140401739
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
newCodePoint = Character.toLowerCase(oldCodepoint);
whitespace = false;
- } else if (Character.isTitleCase(oldCodepoint)) {
- newCodePoint = Character.toLowerCase(oldCodepoint);
- whitespace = false;
- } else if (Character.isLowerCase(oldCodepoint)) {
+ }
+ else if (Character.isLowerCase(oldCodepoint)) {
--- End diff --
Any reason for breaking the line here? Not sure if it's just GitHub UI getting confused, or if the bracket is not in the same line as the else if statement...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:
https://github.com/apache/commons-text/pull/63
You're welcome.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:
https://github.com/apache/commons-text/pull/63
:-)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140401548
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
* <p>This constructor is public to permit tools that require a JavaBean
* instance to operate.</p>
*/
+ @SuppressWarnings("squid:S1118")
--- End diff --
Is that a suppress warning for SonarQube? I am not sure if others are actively looking at the reports produced by SonarQube. I think this is not really necessary, if indeed just for SonarQube.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140526276
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
newCodePoint = Character.toLowerCase(oldCodepoint);
whitespace = false;
- } else if (Character.isTitleCase(oldCodepoint)) {
- newCodePoint = Character.toLowerCase(oldCodepoint);
- whitespace = false;
- } else if (Character.isLowerCase(oldCodepoint)) {
+ }
+ else if (Character.isLowerCase(oldCodepoint)) {
--- End diff --
No, you're right.
My mistake.
Corrected and commited the correction.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by chtompki <gi...@git.apache.org>.
Github user chtompki commented on the issue:
https://github.com/apache/commons-text/pull/63
That last build failure wasn't on you....just fixed it in the master branch.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:
https://github.com/apache/commons-text/pull/63
Well, of course!
And much, much, much more in Java!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140526804
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
* <p>This constructor is public to permit tools that require a JavaBean
* instance to operate.</p>
*/
+ @SuppressWarnings("squid:S1118")
--- End diff --
-1 to this
Eclipse was not displaying any warning before, but with this change it displays this warning: `Unsupported @SuppressWarnings("squid:S1118")`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140528491
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
* <p>This constructor is public to permit tools that require a JavaBean
* instance to operate.</p>
*/
+ @SuppressWarnings("squid:S1118")
--- End diff --
Removed SuppressWarnings anootations completely.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-text/pull/63
Merged in : https://github.com/apache/commons-text/commit/5598109efb2365384b9a9110323ed9a7004cc905 Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140525860
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
--- End diff --
The corresponding lines as the whole method are already completely covered by existing Unit Tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-text/pull/63
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140548121
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,10 +614,7 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
- newCodePoint = Character.toLowerCase(oldCodepoint);
- whitespace = false;
- } else if (Character.isTitleCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
--- End diff --
Removed white space.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140524746
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
* <p>This constructor is public to permit tools that require a JavaBean
* instance to operate.</p>
*/
+ @SuppressWarnings("squid:S1118")
--- End diff --
Well, works with Sonaqube and Intellij IDEA.
Therefore I assume Eclipse and Netbeans recognizing it too.
I guess you want it to removed.
True?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text issue #63: small-code-quality-improvements
Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:
https://github.com/apache/commons-text/pull/63
@chtompki
If you want me to re-push any minor change to re-trigger the build just tell.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-text pull request #63: small-code-quality-improvements
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/63#discussion_r140401636
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
for (int index = 0; index < strLen;) {
final int oldCodepoint = str.codePointAt(index);
final int newCodePoint;
- if (Character.isUpperCase(oldCodepoint)) {
+ if (Character.isUpperCase(oldCodepoint) || Character.isTitleCase(oldCodepoint) ) {
--- End diff --
Nice catch. Any chance you could add a unit test for that too?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org