You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/08/24 14:27:46 UTC
[GitHub] [qpid-protonj2] jiridanek commented on a change in pull request #1: PROTON-2398 Fix typos in production and test code found by running a spellchecker
jiridanek commented on a change in pull request #1:
URL: https://github.com/apache/qpid-protonj2/pull/1#discussion_r694892466
##########
File path: protonj2-client/src/main/java/org/apache/qpid/protonj2/client/impl/ClientMessage.java
##########
@@ -61,6 +61,7 @@
private int messageFormat;
+ // TODO there are no suppliers used in this anywhere; outdated docs?
/**
* Create a new {@link ClientMessage} instance with no default body section or
* section supplier
Review comment:
there are no suppliers used in this anywhere; outdated docs?
##########
File path: protonj2/src/main/java/org/apache/qpid/protonj2/codec/encoders/AbstractDescribedMapTypeEncoder.java
##########
@@ -156,8 +156,8 @@ public void writeArray(ProtonBuffer buffer, EncoderState state, Object[] values)
// Move back and write the size
final int writeSize = buffer.getWriteIndex() - startIndex - Integer.BYTES;
- if (writeSize > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Cannot encode given array, encoded size to large: " + writeSize);
+ if (writeSize > Integer.MAX_VALUE) { // same, always false
Review comment:
condition is always false
##########
File path: protonj2/src/main/java/org/apache/qpid/protonj2/logging/ProtonLogger.java
##########
@@ -88,7 +88,7 @@
public void error(String message);
- public void erro(String message, Object value);
+ public void error(String message, Object value);
Review comment:
public api change
##########
File path: protonj2/src/main/java/org/apache/qpid/protonj2/codec/encoders/AbstractPrimitiveTypeEncoder.java
##########
@@ -50,8 +50,8 @@ public void writeArray(ProtonBuffer buffer, EncoderState state, Object[] values)
// Move back and write the size
long writeSize = buffer.getWriteIndex() - startIndex - Integer.BYTES;
- if (writeSize > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Cannot encode given array, encoded size to large: " + writeSize);
+ if (writeSize > Integer.MAX_VALUE) { // same
Review comment:
condition is always false
##########
File path: pom.xml
##########
@@ -66,9 +66,9 @@
<argLine>-Xmx2g -enableassertions ${jacoco-config}</argLine>
</properties>
- <url>http://qpid.apache.org/proton</url>
+ <url>https://qpid.apache.org/proton</url>
<scm>
- <connection>scm:git:http://git-wip-us.apache.org/repos/asf/qpid-protonj2.git</connection>
+ <connection>scm:git:https://git-wip-us.apache.org/repos/asf/qpid-protonj2.git</connection>
Review comment:
https everywhere, don't see why not
##########
File path: protonj2/src/main/java/org/apache/qpid/protonj2/codec/encoders/AbstractDescribedListTypeEncoder.java
##########
@@ -160,8 +160,8 @@ public void writeArray(ProtonBuffer buffer, EncoderState state, Object[] values)
// Move back and write the size
final int writeSize = buffer.getWriteIndex() - startIndex - Integer.BYTES;
- if (writeSize > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Cannot encode given array, encoded size to large: " + writeSize);
+ if (writeSize > Integer.MAX_VALUE) { // TODO: this condition is always false, except if writeSize was made long maybe?
Review comment:
condition is always false
##########
File path: protonj2-client/src/test/java/org/apache/qpid/protonj2/client/impl/ReceiverTest.java
##########
@@ -1305,15 +1305,16 @@ public void doTtestBlockingReceiveCancelledWhenReceiverClosedOrDetached(boolean
@Test
public void testBlockingReceiveCancelledWhenReceiverRemotelyClosed() throws Exception {
- doTtestBlockingReceiveCancelledWhenReceiverClosedOrDetached(true);
+ doTestBlockingReceiveCancelledWhenReceiverClosedOrDetached(true);
}
@Test
public void testBlockingReceiveCancelledWhenReceiverRemotelyDetached() throws Exception {
- doTtestBlockingReceiveCancelledWhenReceiverClosedOrDetached(false);
+ doTestBlockingReceiveCancelledWhenReceiverClosedOrDetached(false);
}
- public void doTtestBlockingReceiveCancelledWhenReceiverRemotelyClosedOrDetached(boolean close) throws Exception {
+ // TODO unused
+ public void doTestBlockingReceiveCancelledWhenReceiverRemotelyClosedOrDetached(boolean close) throws Exception {
Review comment:
I'll do a fixup commit, the two tests above are obviously calling a wrong do method.
##########
File path: protonj2/src/main/java/org/apache/qpid/protonj2/buffer/ProtonBuffer.java
##########
@@ -1220,7 +1220,7 @@
* {@code src.capacity}, or
* if {@code length} is greater than {@code this.writableBytes}
*/
- ProtonBuffer writeBytes(ProtonBuffer source, int sourrceIndex, int length);
+ ProtonBuffer writeBytes(ProtonBuffer source, int sourceIndex, int length); // TODO rename to offset as in overrides?
Review comment:
The methods in derived classes call this parameter `offset`. Maybe rename it here?
##########
File path: protonj2-client/src/main/java/org/apache/qpid/protonj2/client/TransportOptions.java
##########
@@ -329,20 +329,20 @@ public TransportOptions allowNativeIO(boolean allowNativeIO) {
}
/**
- * @return the nativeIOPeference
+ * @return the nativeIOPreference
*/
- public String[] nativeIOPeference() {
- return nativeIOPeference;
+ public String[] nativeIOPreference() {
+ return nativeIOPreference;
}
/**
- * @param nativeIOPeference the nativeIOPeference to set
+ * @param nativeIOPreference the nativeIOPreference to set
*/
- public void nativeIOPeference(String... nativeIOPeference) {
- if (nativeIOPeference == null || nativeIOPeference.length == 0 || nativeIOPeference.length == 1 && nativeIOPeference[0] == null) {
- this.nativeIOPeference = DEFAULT_NATIVEIO_PREFERENCES;
+ public void nativeIOPreference(String... nativeIOPreference) {
+ if (nativeIOPreference == null || nativeIOPreference.length == 0 || nativeIOPreference.length == 1 && nativeIOPreference[0] == null) {
+ this.nativeIOPreference = DEFAULT_NATIVEIO_PREFERENCES;
} else {
- this.nativeIOPeference = nativeIOPeference;
+ this.nativeIOPreference = nativeIOPreference;
Review comment:
public api change
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org