8263105: security-libs doclint cleanup #2856

bradfordwetmore · 2021-03-06T07:31:09Z

Fix various things pointed out by the most recent doclint run in the security-libs area.

This is docs only: I will be checking doccheck/doclint, and will be running tier1/tier2 tests. Minor spot checks on generated files.

Progress

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

Issue

JDK-8263105: security-libs doclint cleanup

Reviewers

Iris Clark (@irisclark - Reviewer) ⚠️ Review applies to ed0cf2b
Joe Darcy (@jddarcy - Reviewer) ⚠️ Review applies to 1fa29ea
Daniel Fuchs (@dfuch - Reviewer) ⚠️ Review applies to 1fa29ea
Sean Mullan (@seanjmullan - Reviewer) ⚠️ Review applies to ed0cf2b

Download

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

bridgekeeper · 2021-03-06T07:31:32Z

👋 Welcome back wetmore! 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 · 2021-03-06T08:54:13Z

@bradfordwetmore The following labels will be automatically applied to this pull request:

core-libs
security

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.

jonathan-gibbons

I've read the first 10 files. The edits are definitely in the right direction, and will address the "missing comments" issues.

I'll leave it up to you and the others in your team to decide what level of stylistic consistency you would like for the new comments, but just having a relevant comment at all is a great start.

src/java.base/share/classes/java/security/BasicPermission.java

src/java.base/share/classes/java/security/GuardedObject.java

src/java.base/share/classes/java/security/PermissionCollection.java

src/java.base/share/classes/java/security/Permissions.java

mlbridge · 2021-03-09T00:15:16Z

Webrevs

irisclark

I'm happy to see a reduction in the number of warnings.

I suspect there are a few places where Object{In,Out}putStream is fully qualified when it's not strictly necessary; however, since this is in pre-existing code, I don't think this needs to be changed now.

irisclark · 2021-03-09T00:29:20Z

src/java.base/share/classes/java/security/KeyPair.java

    private PrivateKey privateKey;
+
+    /**
+     *


Extra line?

openjdk · 2021-03-09T00:47:15Z

@bradfordwetmore This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8263105: security-libs doclint cleanup

Reviewed-by: iris, darcy, dfuchs, mullan

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 61 new commits pushed to the master branch:

6971c23: 8262351: Extra '0' in java.util.Formatter for '%012a' conversion with a sign character
c6d74bd: 8262910: Cleanup THREAD/TRAPS/naming and typing issues in ObjectMonitor and related code
57f16f9: 8262377: Parallel class resolution loses constant pool error
b482733: 8259218: (fs) Add links in from overloaded methods in java.nio.file.Files
acda812: 8263333: Improve links from core reflection to JLS and JVMS
9399e1b: 8261918: two runtime/cds/appcds/VerifierTest failed with "Unable to use shared archive"
7e52a6e: 8263380: Unintended use of Objects.nonNull in VarHandles
4b5be40: 8238812: assert(false) failed: bad AD file
b2a2ddf: 8262438: sun/security/ssl/SSLLogger/LoggingFormatConsistency.java failed with "SocketException: Socket is closed"
c8c0234: 8262471: Fix coding style in src/java.base/share/classes/java/lang/CharacterDataPrivateUse.java
... and 51 more: https://git.openjdk.java.net/jdk/compare/e1cad97049642ab201d53ff608937f7e7ef3ff3e...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

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

jddarcy

Looks fine.

dfuch

I have recently gone through a similar exercise with java.net: what you have here looks good to me!

src/java.base/share/classes/java/security/CodeSigner.java

src/java.base/share/classes/java/security/Timestamp.java

src/java.base/share/classes/java/security/cert/Certificate.java

src/java.base/share/classes/javax/crypto/SealedObject.java

bradfordwetmore · 2021-03-09T18:20:58Z

/csr needed

openjdk · 2021-03-09T18:21:23Z

@bradfordwetmore the issue for this pull request, JDK-8263105, already has an approved CSR request: JDK-8263207

src/java.base/share/classes/java/security/AllPermission.java

src/java.base/share/classes/java/security/BasicPermission.java

src/java.base/share/classes/java/security/Permissions.java

src/java.base/share/classes/java/security/UnresolvedPermissionCollection.java

src/java.base/share/classes/javax/crypto/SealedObject.java

src/java.base/share/classes/java/security/PrivilegedActionException.java

src/java.security.jgss/share/classes/javax/security/auth/kerberos/KeyImpl.java

bradfordwetmore · 2021-03-11T00:25:46Z

/integrate

openjdk · 2021-03-11T00:26:48Z

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

6971c23: 8262351: Extra '0' in java.util.Formatter for '%012a' conversion with a sign character
c6d74bd: 8262910: Cleanup THREAD/TRAPS/naming and typing issues in ObjectMonitor and related code
57f16f9: 8262377: Parallel class resolution loses constant pool error
b482733: 8259218: (fs) Add links in from overloaded methods in java.nio.file.Files
acda812: 8263333: Improve links from core reflection to JLS and JVMS
9399e1b: 8261918: two runtime/cds/appcds/VerifierTest failed with "Unable to use shared archive"
7e52a6e: 8263380: Unintended use of Objects.nonNull in VarHandles
4b5be40: 8238812: assert(false) failed: bad AD file
b2a2ddf: 8262438: sun/security/ssl/SSLLogger/LoggingFormatConsistency.java failed with "SocketException: Socket is closed"
c8c0234: 8262471: Fix coding style in src/java.base/share/classes/java/lang/CharacterDataPrivateUse.java
... and 51 more: https://git.openjdk.java.net/jdk/compare/e1cad97049642ab201d53ff608937f7e7ef3ff3e...master

Your commit was automatically rebased without conflicts.

Pushed as commit 32cbd19.

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

8263105: security-libs doclint cleanup

1d548df

openjdk bot added security security-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Mar 6, 2021

jonathan-gibbons reviewed Mar 6, 2021

View reviewed changes

bradfordwetmore added 2 commits March 8, 2021 14:13

Only 100 warnings output by default

3da9d34

Final First Draft

f2253d9

bradfordwetmore marked this pull request as ready for review March 9, 2021 00:11

openjdk bot added the rfr Pull request is ready for review label Mar 9, 2021

irisclark approved these changes Mar 9, 2021

View reviewed changes

src/java.base/share/classes/java/security/KeyPair.java Outdated

private PrivateKey privateKey;

/**

*

Copy link

Member

irisclark Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra line?

openjdk bot added the ready Pull request is ready to be integrated label Mar 9, 2021

Codereview Comment

1fa29ea

jddarcy approved these changes Mar 9, 2021

View reviewed changes

dfuch approved these changes Mar 9, 2021

View reviewed changes

seanjmullan reviewed Mar 9, 2021

View reviewed changes

src/java.base/share/classes/java/security/CodeSigner.java Outdated Show resolved Hide resolved

src/java.base/share/classes/java/security/Timestamp.java Outdated Show resolved Hide resolved

src/java.base/share/classes/java/security/cert/Certificate.java Outdated Show resolved Hide resolved

seanjmullan reviewed Mar 9, 2021

View reviewed changes

src/java.base/share/classes/javax/crypto/SealedObject.java Outdated Show resolved Hide resolved

More Codereview Comments

ed0cf2b

RogerRiggs reviewed Mar 10, 2021

View reviewed changes

irisclark approved these changes Mar 10, 2021

View reviewed changes

seanjmullan approved these changes Mar 10, 2021

View reviewed changes

Final codereview comments

edac22b

openjdk bot closed this Mar 11, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated labels Mar 11, 2021

openjdk bot removed the rfr Pull request is ready for review label Mar 11, 2021

bradfordwetmore deleted the JDK-8263105 branch March 18, 2021 22:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8263105: security-libs doclint cleanup #2856

8263105: security-libs doclint cleanup #2856

bradfordwetmore commented Mar 6, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 6, 2021

openjdk bot commented Mar 6, 2021

jonathan-gibbons left a comment •

edited

Loading

mlbridge bot commented Mar 9, 2021 •

edited

Loading

irisclark left a comment

irisclark Mar 9, 2021

openjdk bot commented Mar 9, 2021 •

edited

Loading

jddarcy left a comment

dfuch left a comment

bradfordwetmore commented Mar 9, 2021

openjdk bot commented Mar 9, 2021

bradfordwetmore commented Mar 11, 2021

openjdk bot commented Mar 11, 2021

8263105: security-libs doclint cleanup #2856

8263105: security-libs doclint cleanup #2856

Conversation

bradfordwetmore commented Mar 6, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Mar 6, 2021

openjdk bot commented Mar 6, 2021

jonathan-gibbons left a comment • edited Loading

Choose a reason for hiding this comment

mlbridge bot commented Mar 9, 2021 • edited Loading

Webrevs

irisclark left a comment

Choose a reason for hiding this comment

irisclark Mar 9, 2021

Choose a reason for hiding this comment

openjdk bot commented Mar 9, 2021 • edited Loading

jddarcy left a comment

Choose a reason for hiding this comment

dfuch left a comment

Choose a reason for hiding this comment

bradfordwetmore commented Mar 9, 2021

openjdk bot commented Mar 9, 2021

bradfordwetmore commented Mar 11, 2021

openjdk bot commented Mar 11, 2021

bradfordwetmore commented Mar 6, 2021 •

edited by openjdk bot

Loading

jonathan-gibbons left a comment •

edited

Loading

mlbridge bot commented Mar 9, 2021 •

edited

Loading

openjdk bot commented Mar 9, 2021 •

edited

Loading