-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8247973: Javadoc incorrect for IdentityArrayList, IdentityLinkedList #6694
Conversation
8255675: Typo in java.net.HttpURLConnection
👋 Welcome back anupamdev20! A progress list of the required criteria for merging this PR into |
|
@anupamdev20 The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose removing changes to HttpURLConnection.java
from this PR. Both IdentityArrayList
, IdentityLinkedList
are in java.desktop
module but HttpURLConnection
is in java.base
module. Submit a new issue to fix spelling errors in HttpURLConnection.java
.
@@ -216,8 +216,12 @@ public boolean contains(Object o) { | |||
* Returns the index of the first occurrence of the specified element | |||
* in this list, or -1 if this list does not contain the element. | |||
* More formally, returns the lowest index {@code i} such that | |||
* {@code Objects.equals(o, get(i))}, | |||
* {@code get(i)==o}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall it have spaces on either side of ==
?
* {@code get(i)==o}, | |
* {@code get(i) == o}, |
I see IdentityLinkedList.java
doesn't have spaces around ==
in this case. Yet in this case, probably {@code o == e}
should not have the spaces too.
/** | ||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An extra space.
* Removes the first occurrence of the specified element from this list, | ||
* if it is present. If the list does not contain the element, it is | ||
* if it is present. If this list does not contain the element, it is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* if it is present. If this list does not contain the element, it is | |
* if it is present. If this list does not contain the element, the list is |
I think it adds clarification to what is unchanged.
The documentation for following methods used equals() for object equality:
sun.awt.util.IdentityLinkedList#contains
sun.awt.util.IdentityArrayList#contains
sun.awt.util.IdentityArrayList#indexOf
sun.awt.util.IdentityArrayList#lastIndexOf
sun.awt.util.IdentityArrayList#remove(java.lang.Object)
I have updated the comments to use "==" operator for object equality. Kindly review the changes for the same.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6694/head:pull/6694
$ git checkout pull/6694
Update a local copy of the PR:
$ git checkout pull/6694
$ git pull https://git.openjdk.java.net/jdk pull/6694/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 6694
View PR using the GUI difftool:
$ git pr show -t 6694
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6694.diff