8251496: Fix doclint warnings in jdk.net.httpserver #81

pconcannon · 2020-09-08T16:22:15Z

Hi,

Could someone please review my doc-only fix for JDK-8251496 - ‘Fix doclint warnings in jdk.net.httpserver’ ?

This fix addresses the warnings generated by javadoc -Xdoclint due to missing/incomplete API documentation for several classes within jdk.net.httpserver.

Kind regards,
Patrick

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8251496: Fix doclint warnings in jdk.net.httpserver

Reviewers

Daniel Fuchs (@dfuch - Reviewer)
Roger Riggs (@RogerRiggs - Reviewer)
Chris Hegarty (@ChrisHegarty - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/81/head:pull/81
$ git checkout pull/81

bridgekeeper · 2020-09-08T16:23:57Z

👋 Welcome back pconcannon! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2020-09-08T16:25:17Z

@pconcannon The following label will be automatically applied to this pull request: net.

When this pull request is ready to be reviewed, an RFR email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label (add|remove) "label" command.

mlbridge · 2020-09-08T16:29:33Z

Webrevs

03: Full - Incremental (45303d9)
02: Full - Incremental (660a549c2537aca22a0bca22ce23f67471937767)
01: Full - Incremental (90e7ec0c2e049a483768fe30cee89c347b57a1e2)
00: Full (8faf60769557e8166015150e1252d23427a136ec)

dfuch

Thanks for taking this on Patrick. This mostly looks good to me, with the comments below. Thanks for adding the new tests to cover the added assertion. That's a nice touch 👍

best regards,

-- daniel

src/jdk.httpserver/share/classes/com/sun/net/httpserver/Authenticator.java

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpExchange.java

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpContext.java

src/jdk.httpserver/share/classes/com/sun/net/httpserver/Filter.java

dfuch · 2020-09-08T16:41:32Z

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsParameters.java

 import java.net.InetSocketAddress;
+
 //BEGIN_TIGER_EXCLUDE


I don't have the context here - but if we're removing this import then I guess we should be removing the // BEGIN and // END comments too?

Thanks for spotting this. IntelliJ re-arranged the imports automatically. I reverted it back to its original state

I suspect the commented lines are obsolete TIGER was a long time ago.
That looks like some markup for a tool to use a non-ssl version as a demo.
@AlanBateman Alan might know.

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsParameters.java

dfuch · 2020-09-08T16:50:24Z

/csr needed

openjdk · 2020-09-08T16:50:49Z

@dfuch this pull request will not be integrated until the CSR request JDK-8252585 for issue JDK-8251496 has been approved.

RogerRiggs

The @param and @return lines that start with a capital letter seem out of place.
The contents of most @tags are usually phrases without capitalization or period.

src/jdk.httpserver/share/classes/com/sun/net/httpserver/Authenticator.java

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpPrincipal.java

RogerRiggs · 2020-09-09T13:27:02Z

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsParameters.java

 import java.net.InetSocketAddress;
+
 //BEGIN_TIGER_EXCLUDE


I suspect the commented lines are obsolete TIGER was a long time ago.
That looks like some markup for a tool to use a non-ssl version as a demo.
@AlanBateman Alan might know.

ChrisHegarty · 2020-09-09T14:19:47Z

@RogerRiggs There are many many formatting and cleanup tasks that could be done to this particular code. My advise to Patrick was (and continues to be) to just focus on the particular issue at hand - clean up the doclint warnings. Other annoyances can be fixed up later during another pass. This issue is already overloaded with cleanup and semantic spec changes. It will just increase the cognitive complexity to continue to fix (arguably) unrelated annoyances.

openjdk · 2020-09-09T14:24:35Z

@pconcannon This change now passes all automated pre-integration checks. In addition to the automated checks, the change must also fulfill all project specific requirements

After integration, the commit message will be:

8251496: Fix doclint warnings in jdk.net.httpserver

Reviewed-by: dfuchs, rriggs, chegar

If you would like to add a summary, use the /summary command.
To credit additional contributors, use the /contributor command.
To add additional solved issues, use the /issue command.

Since the source branch of this PR was last updated there have been 129 commits pushed to the master branch:

b9729cb: 8253284: Zero OrderAccess barrier mappings are incorrect
284bbf0: 8253079: DeterministicDump.java fails due to garbage in structure padding
a4affd5: 8253412: Unsupported GC options passed in JAVA_TOOL_OPTIONS are silently ignored
96f722c: 8252114: Windows-AArch64: Enable and test ZGC and ShenandoahGC
f7b1ce4: 8253447: Remove buggy code introduced by 8249451
282b9dc: 8081833: Clean up JVMFlag getter/setter code
0e98fc1: 8253237: [REDO] Improve large object handling during evacuation
d1f9b8a: 8234808: jdb quoted option parsing broken
d8921ed: 8247281: migrate ObjectMonitor::_object to OopStorage
f800af9: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
... and 119 more: https://git.openjdk.java.net/jdk/compare/8db333531fabbe95461c00c2ba26191ce39442cd...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate b9729cb43202eb65a9d08aecc4eb8f63b744f9e0.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpExchange.java

…tion

…ders; to be dealt with in separate issue (JDK-8253005)

pconcannon · 2020-09-22T08:55:22Z

/integrate

openjdk · 2020-09-22T08:56:29Z

@pconcannon Since your change was applied there have been 129 commits pushed to the master branch:

b9729cb: 8253284: Zero OrderAccess barrier mappings are incorrect
284bbf0: 8253079: DeterministicDump.java fails due to garbage in structure padding
a4affd5: 8253412: Unsupported GC options passed in JAVA_TOOL_OPTIONS are silently ignored
96f722c: 8252114: Windows-AArch64: Enable and test ZGC and ShenandoahGC
f7b1ce4: 8253447: Remove buggy code introduced by 8249451
282b9dc: 8081833: Clean up JVMFlag getter/setter code
0e98fc1: 8253237: [REDO] Improve large object handling during evacuation
d1f9b8a: 8234808: jdb quoted option parsing broken
d8921ed: 8247281: migrate ObjectMonitor::_object to OopStorage
f800af9: 8241390: 'Deadlock' with VM_RedefineClasses::lock_classes()
... and 119 more: https://git.openjdk.java.net/jdk/compare/8db333531fabbe95461c00c2ba26191ce39442cd...master

Your commit was automatically rebased without conflicts.

Pushed as commit ae20dd6.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

…sent (openjdk#81) Also convert keys to custom Key object to have more fine grained control over constructor, equals, and hashCode methods. Integer::valueOf pulls from a global cache which makes PEA optimizations harder.

openjdk bot added the rfr Pull request is ready for review label Sep 8, 2020

openjdk bot added the net net-dev@openjdk.org label Sep 8, 2020

dfuch suggested changes Sep 8, 2020

View reviewed changes

openjdk bot added the csr Pull request needs approved CSR before integration label Sep 8, 2020

RogerRiggs suggested changes Sep 9, 2020

View reviewed changes

RogerRiggs approved these changes Sep 9, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 9, 2020

ChrisHegarty reviewed Sep 9, 2020

View reviewed changes

src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpExchange.java Outdated Show resolved Hide resolved

pconcannon added 2 commits September 10, 2020 12:38

8251496: Fix doclint warnings in jdk.net.httpserver

1886a3c

8251496: reworded abstract cstr comments; removed unnecessary punctua…

0278de9

…tion

pconcannon force-pushed the JDK-8251496 branch from 660a549 to 0278de9 Compare September 10, 2020 11:38

8251496: removed @throws IOException from HttpExchange.sendRequestHea…

45303d9

…ders; to be dealt with in separate issue (JDK-8253005)

ChrisHegarty approved these changes Sep 10, 2020

View reviewed changes

dfuch approved these changes Sep 10, 2020

View reviewed changes

RogerRiggs approved these changes Sep 10, 2020

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label Sep 14, 2020

openjdk bot closed this Sep 22, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 22, 2020

pconcannon deleted the JDK-8251496 branch September 22, 2020 09:23

cushon pushed a commit to cushon/jdk that referenced this pull request Apr 2, 2021

Annotate toArray in subtypes of Collection (openjdk#81)

2220900

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8251496: Fix doclint warnings in jdk.net.httpserver #81

8251496: Fix doclint warnings in jdk.net.httpserver #81

pconcannon commented Sep 8, 2020 •

edited by openjdk bot

Loading

bridgekeeper bot commented Sep 8, 2020

openjdk bot commented Sep 8, 2020

mlbridge bot commented Sep 8, 2020 •

edited

Loading

dfuch left a comment

dfuch Sep 8, 2020

pconcannon Sep 9, 2020

RogerRiggs Sep 9, 2020

dfuch commented Sep 8, 2020

openjdk bot commented Sep 8, 2020

RogerRiggs left a comment

RogerRiggs Sep 9, 2020

ChrisHegarty commented Sep 9, 2020

openjdk bot commented Sep 9, 2020 •

edited

Loading

pconcannon commented Sep 22, 2020

openjdk bot commented Sep 22, 2020

8251496: Fix doclint warnings in jdk.net.httpserver #81

8251496: Fix doclint warnings in jdk.net.httpserver #81

Conversation

pconcannon commented Sep 8, 2020 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Sep 8, 2020

openjdk bot commented Sep 8, 2020

mlbridge bot commented Sep 8, 2020 • edited Loading

Webrevs

dfuch left a comment

Choose a reason for hiding this comment

dfuch Sep 8, 2020

Choose a reason for hiding this comment

pconcannon Sep 9, 2020

Choose a reason for hiding this comment

RogerRiggs Sep 9, 2020

Choose a reason for hiding this comment

dfuch commented Sep 8, 2020

openjdk bot commented Sep 8, 2020

RogerRiggs left a comment

Choose a reason for hiding this comment

RogerRiggs Sep 9, 2020

Choose a reason for hiding this comment

ChrisHegarty commented Sep 9, 2020

openjdk bot commented Sep 9, 2020 • edited Loading

pconcannon commented Sep 22, 2020

openjdk bot commented Sep 22, 2020

pconcannon commented Sep 8, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Sep 8, 2020 •

edited

Loading

openjdk bot commented Sep 9, 2020 •

edited

Loading