On Tue, 3 Jun 2025 13:15:33 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
> I think the way it currently is follows the builder pattern better. All of > the 'set' methods return `this` which means if I change this method to a > constructor, I'd have to duplicate the "set" code from all those methods. I'm not sure what you mean - can you give an example? I don't see any advantages of using a static method here, but I agree it works either way. > I like the idea of using `Duration` but the API for it doesn't really lend > itself to this use case. When the certificate is finally created, we write > `Date` objects to the DerOutputStream so I'd have to choose a start time and > then calculate the end time based on the duration. It's not hard, but it > doesn't seem worth it when the common use case of this class is to generate > short-lived certificates for a test. The default one-hour validity will cover > the vast majority of tests. I view this test utility class as being flexible enough for different cases. This is a useful method in that the other parameters are common fields that all certs usually have but it also means I can't use this method to to create a certificate with a longer, or shorter validity period. Alternatively, how about adding another method that hard-codes it as one hour: `CertificateBuilder oneHourValidity()` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23700#discussion_r2124254602
