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

Reply via email to