On Fri, 19 Nov 2021 19:50:33 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Martin Balao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> P11Key static inner classes refactorings. > > Hmm, thinking more about "internal"/"opaque", given this is naming for the > parent, maybe "internal" is more correct. The non-sensitive keys are > encapsulated by the children classes and is still an instance of the parent. > If you name the parent class w/ "opaque" suffix, it looks > misleading/confusing. The opaqueness is implied by the implementation instead > of the properties of the objects. > Hi @valeriepeng , > > Yes, I just verified how serialization works for P11Keys and your 'transient' > change makes sense to me now. > > I see your point about Internal/Opaque. Keep 'Internal' then if you prefer. > The whole inheritance relationship between these classes sounds a bit weird > to me, beyond the names we call them. I wouldn't suggest any additional > changes there now, though. > > It looks to me that the only 2 changes expected for Webrev.02 are: 1) > P11RSAPrivateNonCRTKey to use the parent's protected 'n'; and 2) removal of > 'long errorCode = e.getErrorCode();'. > > Now that we almost have the changeset ready, I'm not sure of how to proceed > with the push. Do you want me to commit Webrev.02 in my own branch and use > the 'Co-authored-by:' header? If we do that, do we still need an additional > Reviewer? Do you prefer to have your own branch? Please let me know of how to > move forward. > > Martin.- You can just incorporate the changes on your branch and proceeds with me being the reviewer. The webrev that I sent u is just an easier way to express the comments/feedbacks. Thanks, Valerie ------------- PR: https://git.openjdk.java.net/jdk/pull/4961