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 2020/03/27 03:48:02 UTC
[GitHub] [commons-collections] dota17 opened a new pull request #144: Update
the JavaDoc of the BidiMap
dota17 opened a new pull request #144: Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] dota17 commented on issue #144:
[COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
dota17 commented on issue #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#issuecomment-615144805
Thanks for the response @kinow @garydgregory .
Keep the code instead of the link ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] kinow commented on a change in pull request
#144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r410519550
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/DualLinkedHashBidiMap.java
##########
@@ -26,9 +26,9 @@
import org.apache.commons.collections4.BidiMap;
/**
- * Implementation of {@code BidiMap} that uses two {@code LinkedHashMap} instances.
+ * Implementation of {@link BidiMap} that uses two {@link LinkedHashMap} instances.
Review comment:
I found the document I mentioned earlier. It was not exactly a presentation, but the Oracle docs for javadocs: https://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide (where it says "Use in-line links economically"). There's also a similar style guide in UI/UX, to avoid cluttering user interfaces. However, I'm fine whichever way :+1:
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r410220989
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/TreeBidiMap.java
##########
@@ -782,7 +782,7 @@ private static boolean isBlack(final Node<?, ?> node, final DataElement dataElem
}
/**
- * force a node (if it exists) red
+ * Force a node (if it exists) red.
Review comment:
If a method starts with "make...", it usually reads better to start the Javadoc with "Makes..." IMO, It's confusing to have the Javadoc be different, otherwise you could say that all method that start with "set..." also "forces" a value.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] coveralls edited a comment on issue #144:
[COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#issuecomment-604803773
[](https://coveralls.io/builds/29676456)
Coverage increased (+0.001%) to 90.007% when pulling **f37725a9bd50be1f4602f8718b62e91b5644c285 on dota17:UpdateBidiMapJavaDoc** into **a02a0e699379242f91964354adaf8c44a65d03dc 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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r400239117
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/AbstractDualBidiMap.java
##########
@@ -620,7 +617,7 @@ public void remove() {
private static final long serialVersionUID = 4040410962603292348L;
/**
- * Constructs a new view of the BidiMap.
+ * Constructs a new view of the BidiMap
Review comment:
-1: A sentence ends with a period.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] kinow commented on a change in pull request
#144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r399619219
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/AbstractDualBidiMap.java
##########
@@ -620,7 +617,7 @@ public void remove() {
private static final long serialVersionUID = 4040410962603292348L;
/**
- * Constructs a new view of the BidiMap.
+ * Constructs a new view of the BidiMap
Review comment:
I think we have some other statements in the code base that end in a period. Any specific reason to remove them? If not, I think it would be simpler to just leave them as-is.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r410219387
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/DualLinkedHashBidiMap.java
##########
@@ -26,9 +26,9 @@
import org.apache.commons.collections4.BidiMap;
/**
- * Implementation of {@code BidiMap} that uses two {@code LinkedHashMap} instances.
+ * Implementation of {@link BidiMap} that uses two {@link LinkedHashMap} instances.
Review comment:
That's silly IMO, these links are fine.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r410221696
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/TreeBidiMap.java
##########
@@ -1230,8 +1230,8 @@ private void swapPosition(final Node<K, V> x, final Node<K, V> y, final DataElem
}
/**
- * check if an object is fit to be proper input ... has to be
- * Comparable and non-null
+ * Check if an object is fit to be proper input ... has to be
Review comment:
"Check" -> "Checks"
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] coveralls commented on issue #144: Update the
JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #144: Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#issuecomment-604803773
[](https://coveralls.io/builds/29652373)
Coverage remained the same at 90.005% when pulling **86bab4742383e683448bbb3d263779cb85e556d2 on dota17:UpdateBidiMapJavaDoc** into **a02a0e699379242f91964354adaf8c44a65d03dc 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
With regards,
Apache Git Services
[GitHub] [commons-collections] kinow commented on a change in pull request
#144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r399619466
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/DualLinkedHashBidiMap.java
##########
@@ -26,9 +26,9 @@
import org.apache.commons.collections4.BidiMap;
/**
- * Implementation of {@code BidiMap} that uses two {@code LinkedHashMap} instances.
+ * Implementation of {@link BidiMap} that uses two {@link LinkedHashMap} instances.
Review comment:
This might have been intentional? Some people prefer to avoid adding too many links in the Javadoc (forgot where I saw a presentation about that, but it was in some guide for writing Javadocs where the author mentioned to keep the number of links low, so users are not distracted).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] dota17 closed pull request #144:
[COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
dota17 closed pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] coveralls edited a comment on issue #144:
[COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#issuecomment-604803773
[](https://coveralls.io/builds/29758274)
Coverage increased (+0.001%) to 90.007% when pulling **43a2d9915aa4cdfe040d25c8c28e5326ea8e36e9 on dota17:UpdateBidiMapJavaDoc** into **a02a0e699379242f91964354adaf8c44a65d03dc 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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r400240088
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/AbstractDualBidiMap.java
##########
@@ -497,7 +494,7 @@ public boolean remove(final Object key) {
protected boolean canRemove = false;
/**
- * Constructor.
+ * Constructor a KeySetIterator of the BidiMap
Review comment:
"Constructor." or "Constructs a new instance." is a good place holder comment, otherwise, you can describe what kind of initialization takes place.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] coveralls edited a comment on issue #144:
[COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#issuecomment-604803773
[](https://coveralls.io/builds/29676754)
Coverage increased (+0.001%) to 90.007% when pulling **9d7629cfa77292f23d292486415b234457bec291 on dota17:UpdateBidiMapJavaDoc** into **a02a0e699379242f91964354adaf8c44a65d03dc 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
With regards,
Apache Git Services
[GitHub] [commons-collections] garydgregory commented on a change in pull
request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r410221467
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/TreeBidiMap.java
##########
@@ -1130,7 +1130,7 @@ private void doRedBlackDeleteFixup(final Node<K, V> replacementNode, final DataE
}
/**
- * swap two nodes (except for their content), taking care of
+ * Swap two nodes (except for their content), taking care of
Review comment:
"Swap" -> "Swaps"
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] kinow commented on a change in pull request
#144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r399619381
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/AbstractDualBidiMap.java
##########
@@ -452,7 +449,7 @@ public void clear() {
private static final long serialVersionUID = -7107935777385040694L;
/**
- * Constructs a new view of the BidiMap.
+ * Constructs a new KeySet of the BidiMap
Review comment:
I think the view it referred to was more in an abstract way, but I think it should be fine to be specific and mention the `KeySet` type here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [commons-collections] kinow commented on a change in pull request
#144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #144: [COLLECTIONS-757] Update the JavaDoc of the BidiMap
URL: https://github.com/apache/commons-collections/pull/144#discussion_r399619118
##########
File path: src/main/java/org/apache/commons/collections4/bidimap/AbstractDualBidiMap.java
##########
@@ -497,7 +494,7 @@ public boolean remove(final Object key) {
protected boolean canRemove = false;
/**
- * Constructor.
+ * Constructor a KeySetIterator of the BidiMap
Review comment:
"Constructor of a" or "Constructs a" would sound better I think.
However, I think just "Constructor" is much simpler/easier to maintain (ditto for the other similar changes in this PR)
----------------------------------------------------------------
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
With regards,
Apache Git Services