Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v4]

2022-08-07 Thread Sean Mullan
On Fri, 5 Aug 2022 21:49:01 GMT, Mark Powers wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > Mark Powers has updated the pull request incrementally with one additional > commit since the last revision: > > comment applies to two files Marked as reviewed by mullan (Reviewer). -

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v4]

2022-08-05 Thread Mark Powers
On Fri, 5 Aug 2022 21:49:01 GMT, Mark Powers wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > Mark Powers has updated the pull request incrementally with one additional > commit since the last revision: > > comment applies to two files Thanks Sean and Brad for the review! -

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v4]

2022-08-05 Thread Bradford Wetmore
On Fri, 5 Aug 2022 21:49:01 GMT, Mark Powers wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > Mark Powers has updated the pull request incrementally with one additional > commit since the last revision: > > comment applies to two files LGTM. - Marked as reviewed by wet

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v4]

2022-08-05 Thread Mark Powers
> https://bugs.openjdk.org/browse/JDK-8290975 Mark Powers has updated the pull request incrementally with one additional commit since the last revision: comment applies to two files - Changes: - all: https://git.openjdk.org/jdk/pull/9664/files - new: https://git.openjdk.org/j

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v3]

2022-08-05 Thread Mark Powers
> https://bugs.openjdk.org/browse/JDK-8290975 Mark Powers has updated the pull request incrementally with one additional commit since the last revision: comment from sean - Changes: - all: https://git.openjdk.org/jdk/pull/9664/files - new: https://git.openjdk.org/jdk/pull/966

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Bradford Wetmore
On Fri, 5 Aug 2022 14:09:18 GMT, Mark Powers wrote: >> Yes but `defaultName` (like `prompt`) should never change once the object is >> constructed. So it seems inconsistent not to also mark it final (and if >> necessary, set it to a default value (`null`) in the constructors). > > Agreed. I'll

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Mark Powers
On Fri, 5 Aug 2022 14:29:38 GMT, Mark Powers wrote: >> I'll file a bug later today and add to this bug report. > > You must mean sun.security.util.Debug. JDK-8291974 - PR: https://git.openjdk.org/jdk/pull/9664

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Mark Powers
On Thu, 4 Aug 2022 20:20:35 GMT, Mark Powers wrote: >> src/java.base/share/classes/javax/security/auth/PrivateCredentialPermission.java >> line 130: >> >>> 128: * @serial >>> 129: */ >>> 130: private final boolean testing = false; >> >> This should really use java.security.debug

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Mark Powers
On Fri, 5 Aug 2022 10:04:43 GMT, Sean Mullan wrote: >> I verified the error message happens when `defaultName` is final. > > Yes but `defaultName` (like `prompt`) should never change once the object is > constructed. So it seems inconsistent not to also mark it final (and if > necessary, set it

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Sean Mullan
On Thu, 4 Aug 2022 17:03:37 GMT, Bradford Wetmore wrote: >> src/java.base/share/classes/javax/security/auth/callback/TextInputCallback.java >> line 46: >> >>> 44: * @since 1.4 >>> 45: */ >>> 46: private final String prompt; >> >> I think you can also mark `defaultText` final. > >

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-05 Thread Sean Mullan
On Thu, 4 Aug 2022 20:21:44 GMT, Mark Powers wrote: >> First constructor doesn't set defaultName (or inputName), so there will be a >> error "might not have been initialized". > > I verified the error message happens when `defaultName` is final. Yes but `defaultName` (like `prompt`) should neve

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security [v2]

2022-08-04 Thread Mark Powers
> https://bugs.openjdk.org/browse/JDK-8290975 Mark Powers has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - just one more thing - Merge - sean and brad comments - second iteration - first iteration - C

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Mark Powers
On Thu, 4 Aug 2022 21:32:04 GMT, Bradford Wetmore wrote: >> IJ complains about unnecessary imports and asks If I want to let it >> automatically optimize them. I always answer "yes". I'll fix as you suggest. > > The default setup wants to put the sun.* imports above the java.*. The > default o

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Bradford Wetmore
On Thu, 4 Aug 2022 20:21:17 GMT, Mark Powers wrote: >> src/java.base/share/classes/javax/security/auth/Subject.java line 28: >> >>> 26: package javax.security.auth; >>> 27: >>> 28: import sun.security.util.ResourcesMgr; >> >> I suggest putting the internal imports below the java.* or other st

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Bradford Wetmore
On Thu, 4 Aug 2022 20:26:11 GMT, Mark Powers wrote: >> src/java.base/share/classes/javax/security/auth/Subject.java line 36: >> >>> 34: import java.security.*; >>> 35: import java.text.MessageFormat; >>> 36: import java.util.*; >> >> What is the style convention you're trying to conform to here

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Mark Powers
On Thu, 4 Aug 2022 16:45:36 GMT, Bradford Wetmore wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > src/java.base/share/classes/javax/security/auth/Subject.java line 36: > >> 34: import java.security.*; >> 35: import java.text.MessageFormat; >> 36: import java.util.*; > > What is the st

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Mark Powers
On Wed, 3 Aug 2022 14:37:20 GMT, Sean Mullan wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > src/java.base/share/classes/javax/security/auth/PrivateCredentialPermission.java > line 130: > >> 128: * @serial >> 129: */ >> 130: private final boolean testing = false; > > Th

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Mark Powers
On Thu, 4 Aug 2022 17:02:48 GMT, Bradford Wetmore wrote: >> src/java.base/share/classes/javax/security/auth/callback/NameCallback.java >> line 45: >> >>> 43: * @since 1.4 >>> 44: */ >>> 45: private final String prompt; >> >> I think you can also mark `defaultName` final. > > Firs

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Bradford Wetmore
On Wed, 3 Aug 2022 14:49:25 GMT, Sean Mullan wrote: >> https://bugs.openjdk.org/browse/JDK-8290975 > > src/java.base/share/classes/javax/security/auth/callback/NameCallback.java > line 45: > >> 43: * @since 1.4 >> 44: */ >> 45: private final String prompt; > > I think you can als

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-04 Thread Bradford Wetmore
On Wed, 27 Jul 2022 20:24:19 GMT, Mark Powers wrote: > https://bugs.openjdk.org/browse/JDK-8290975 LMGTM, don't need to rereview, but would appreciate comments on the import style you used. src/java.base/share/classes/javax/security/auth/Subject.java line 36: > 34: import java.security.*; > 3

Re: RFR: JDK-8290975 Minor cleanup could be done in javax.security

2022-08-03 Thread Sean Mullan
On Wed, 27 Jul 2022 20:24:19 GMT, Mark Powers wrote: > https://bugs.openjdk.org/browse/JDK-8290975 src/java.base/share/classes/javax/security/auth/PrivateCredentialPermission.java line 130: > 128: * @serial > 129: */ > 130: private final boolean testing = false; This should real