You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/04/20 05:23:45 UTC
[GitHub] [commons-text] arturobernalg opened a new pull request #225: TEXT-202 - Extract duplicate code
arturobernalg opened a new pull request #225:
URL: https://github.com/apache/commons-text/pull/225
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] kinow commented on pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #225:
URL: https://github.com/apache/commons-text/pull/225#issuecomment-828381214
Merged, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] kinow commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r616359101
##########
File path: src/main/java/org/apache/commons/text/StrBuilder.java
##########
@@ -319,15 +319,14 @@ public StrBuilder append(final boolean value) {
buffer[size++] = 't';
buffer[size++] = 'r';
buffer[size++] = 'u';
- buffer[size++] = 'e';
} else {
ensureCapacity(size + 5);
buffer[size++] = 'f';
buffer[size++] = 'a';
buffer[size++] = 'l';
buffer[size++] = 's';
- buffer[size++] = 'e';
}
+ buffer[size++] = 'e';
Review comment:
I prefer the other version, even though this one indeed reduces the duplication, the other version clearly shows that we have `true` and `false`, without the dev having to think why `tru` and `fals`, just to find the `e` was set out the `if/else` statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] arturobernalg commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r617190541
##########
File path: src/main/java/org/apache/commons/text/StrTokenizer.java
##########
@@ -842,10 +838,9 @@ private int readWithQuotes(final char[] srcChars, final int start, final int len
continue;
}
- // copy regular character from outside quotes
- workArea.append(srcChars[pos++]);
- trimStart = workArea.size();
}
+ workArea.append(srcChars[pos++]);
Review comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] arturobernalg commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r617190574
##########
File path: src/main/java/org/apache/commons/text/StringTokenizer.java
##########
@@ -848,11 +844,9 @@ private int readWithQuotes(final char[] srcChars, final int start, final int len
pos += trimmedLen;
continue;
}
-
- // copy regular character from outside quotes
- workArea.append(srcChars[pos++]);
- trimStart = workArea.size();
}
+ workArea.append(srcChars[pos++]);
Review comment:
changed. TY
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] coveralls edited a comment on pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #225:
URL: https://github.com/apache/commons-text/pull/225#issuecomment-823001272
[](https://coveralls.io/builds/38958933)
Coverage decreased (-0.002%) to 97.979% when pulling **69e8cfb40596b921e333f8efebb91b4e93254b33 on arturobernalg:feature/TEXT-202** into **857a3ed9eeda0b197ebe57035d7de52cdf551b60 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] coveralls commented on pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #225:
URL: https://github.com/apache/commons-text/pull/225#issuecomment-823001272
[](https://coveralls.io/builds/38958393)
Coverage decreased (-0.003%) to 97.979% when pulling **38c1739984d2b7709cd8b63b051db5610114440e on arturobernalg:feature/TEXT-202** into **857a3ed9eeda0b197ebe57035d7de52cdf551b60 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] arturobernalg commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r616378178
##########
File path: src/main/java/org/apache/commons/text/StrBuilder.java
##########
@@ -319,15 +319,14 @@ public StrBuilder append(final boolean value) {
buffer[size++] = 't';
buffer[size++] = 'r';
buffer[size++] = 'u';
- buffer[size++] = 'e';
} else {
ensureCapacity(size + 5);
buffer[size++] = 'f';
buffer[size++] = 'a';
buffer[size++] = 'l';
buffer[size++] = 's';
- buffer[size++] = 'e';
}
+ buffer[size++] = 'e';
Review comment:
HI @kinow
sound fair. Changed
TY
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] kinow closed pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
kinow closed pull request #225:
URL: https://github.com/apache/commons-text/pull/225
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] garydgregory commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r616587084
##########
File path: src/main/java/org/apache/commons/text/StrBuilder.java
##########
@@ -319,15 +319,14 @@ public StrBuilder append(final boolean value) {
buffer[size++] = 't';
buffer[size++] = 'r';
buffer[size++] = 'u';
- buffer[size++] = 'e';
} else {
ensureCapacity(size + 5);
buffer[size++] = 'f';
buffer[size++] = 'a';
buffer[size++] = 'l';
buffer[size++] = 's';
- buffer[size++] = 'e';
}
+ buffer[size++] = 'e';
Review comment:
I agree with @kinow, you have to think about maintainers reading the code, not just applying changes mechanically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] coveralls edited a comment on pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #225:
URL: https://github.com/apache/commons-text/pull/225#issuecomment-823001272
[](https://coveralls.io/builds/38993996)
Coverage decreased (-0.002%) to 97.979% when pulling **5a793fdde5e5e83fb9a02f8108017baaa8e945b3 on arturobernalg:feature/TEXT-202** into **857a3ed9eeda0b197ebe57035d7de52cdf551b60 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] coveralls edited a comment on pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #225:
URL: https://github.com/apache/commons-text/pull/225#issuecomment-823001272
[](https://coveralls.io/builds/38994005)
Coverage decreased (-0.002%) to 97.979% when pulling **5a793fdde5e5e83fb9a02f8108017baaa8e945b3 on arturobernalg:feature/TEXT-202** into **857a3ed9eeda0b197ebe57035d7de52cdf551b60 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] arturobernalg commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r616626079
##########
File path: src/main/java/org/apache/commons/text/StrBuilder.java
##########
@@ -319,15 +319,14 @@ public StrBuilder append(final boolean value) {
buffer[size++] = 't';
buffer[size++] = 'r';
buffer[size++] = 'u';
- buffer[size++] = 'e';
} else {
ensureCapacity(size + 5);
buffer[size++] = 'f';
buffer[size++] = 'a';
buffer[size++] = 'l';
buffer[size++] = 's';
- buffer[size++] = 'e';
}
+ buffer[size++] = 'e';
Review comment:
Yep. Agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-text] kinow commented on a change in pull request #225: TEXT-202 - Extract duplicate code
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #225:
URL: https://github.com/apache/commons-text/pull/225#discussion_r616997571
##########
File path: src/main/java/org/apache/commons/text/StringTokenizer.java
##########
@@ -848,11 +844,9 @@ private int readWithQuotes(final char[] srcChars, final int start, final int len
pos += trimmedLen;
continue;
}
-
- // copy regular character from outside quotes
- workArea.append(srcChars[pos++]);
- trimStart = workArea.size();
}
+ workArea.append(srcChars[pos++]);
Review comment:
Ditto here about the comment that was removed :+1:
##########
File path: src/main/java/org/apache/commons/text/StrTokenizer.java
##########
@@ -842,10 +838,9 @@ private int readWithQuotes(final char[] srcChars, final int start, final int len
continue;
}
- // copy regular character from outside quotes
- workArea.append(srcChars[pos++]);
- trimStart = workArea.size();
}
+ workArea.append(srcChars[pos++]);
Review comment:
@arturobernalg can you add back the comment above these lines, please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org