You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "rajucomp (via GitHub)" <gi...@apache.org> on 2024/02/29 23:37:38 UTC
[PR] Minor Grammatical changes and some minor code changes. [commons-dbcp]
rajucomp opened a new pull request, #355:
URL: https://github.com/apache/commons-dbcp/pull/355
(no comment)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Minor Grammatical changes and some minor code changes. [commons-dbcp]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory closed pull request #355: Minor Grammatical changes and some minor code changes.
URL: https://github.com/apache/commons-dbcp/pull/355
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Minor Grammatical changes and some minor code changes. [commons-dbcp]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #355:
URL: https://github.com/apache/commons-dbcp/pull/355#issuecomment-2297049335
Closing, no reply.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Minor Grammatical changes and some minor code changes. [commons-dbcp]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #355:
URL: https://github.com/apache/commons-dbcp/pull/355#discussion_r1511983504
##########
src/main/java/org/apache/commons/dbcp2/DataSourceConnectionFactory.java:
##########
@@ -16,6 +16,8 @@
*/
package org.apache.commons.dbcp2;
+import static java.util.Objects.isNull;
Review Comment:
We only use static imports for JUnit assertions.
##########
src/main/java/org/apache/commons/dbcp2/AbandonedTrace.java:
##########
@@ -169,7 +169,7 @@ private void init(final AbandonedTrace parent) {
*/
protected void removeThisTrace(final Object source) {
if (source instanceof AbandonedTrace) {
- AbandonedTrace.class.cast(source).removeTrace(this);
+ ((AbandonedTrace) source).removeTrace(this);
Review Comment:
Why is this change needed?
##########
src/main/java/org/apache/commons/dbcp2/BasicDataSource.java:
##########
@@ -297,7 +297,7 @@ protected static void validateConnectionFactory(final PoolableConnectionFactory
private String connectionString;
/**
- * The connection user name to be passed to our JDBC driver to establish a connection.
+ * The connection username to be passed to our JDBC driver to establish a connection.
Review Comment:
Let's not use compound nouns when we don't have to.
##########
src/main/java/org/apache/commons/dbcp2/DelegatingDatabaseMetaData.java:
##########
@@ -985,9 +985,6 @@ public ResultSet getVersionColumns(final String catalog, final String schema, fi
}
protected void handleException(final SQLException e) throws SQLException {
- if (connection == null) {
Review Comment:
This might break subclasses, so don't change it. Unlikely, but possible.
##########
src/main/java/org/apache/commons/dbcp2/BasicDataSource.java:
##########
@@ -1794,7 +1794,7 @@ public void setConnectionFactoryClassName(final String connectionFactoryClassNam
*/
public void setConnectionInitSqls(final Collection<String> connectionInitSqls) {
final List<String> collect = Utils.isEmpty(connectionInitSqls) ? null
- : connectionInitSqls.stream().filter(s -> !isEmpty(s)).collect(Collectors.toList());
+ : connectionInitSqls.stream().filter(s -> !isEmpty(s)).collect(Collectors.toList());
Review Comment:
Please don't change the formatting, it makes PR noisier and takes longer to review.
##########
src/main/java/org/apache/commons/dbcp2/BasicDataSource.java:
##########
@@ -1494,7 +1494,7 @@ public Duration getValidationQueryTimeoutDuration() {
* @throws IllegalStateException if invalidating the connection failed.
* @since 2.1
*/
- @SuppressWarnings("resource")
+ @SuppressWarnings({"resource", "RedundantSuppression"})
Review Comment:
Not needed.
##########
src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java:
##########
@@ -440,10 +448,10 @@ private void validatePropertyNames(final Reference ref, final Name name, final L
// and it is not in the "silent" list, tell user we are ignoring it.
if (!(ALL_PROPERTY_NAMES.contains(propertyName) || NUPROP_WARNTEXT.containsKey(propertyName) || SILENT_PROPERTIES.contains(propertyName))) {
final String propertyValue = Objects.toString(ra.getContent(), null);
- final StringBuilder stringBuilder = new StringBuilder(nameString);
- stringBuilder.append("Ignoring unknown property: ").append("value of \"").append(propertyValue).append("\" for \"").append(propertyName)
- .append("\" property");
- infoMessages.add(stringBuilder.toString());
+ final String stringBuilder =
Review Comment:
Why?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org