On Thu, 27 Mar 2025 13:33:37 GMT, Fernando Guallini <fguall...@openjdk.org>
wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> minor: imports fix
>
> test/lib/jdk/test/lib/security/artifacts/ThirdPartyArtifacts.java line 30:
>
>> 28: import jdk.test.lib.artifacts.ArtifactResolverException;
>> 29:
>> 30: import java.io.IOException;
>
> nit: unused imports can be removed
done in the next commit
> test/lib/jdk/test/lib/security/artifacts/ThirdPartyArtifacts.java line 35:
>
>> 33: public class ThirdPartyArtifacts {
>> 34:
>> 35: public static final String ACVP_BUNDLE_LOC = "jpg.tests.jdk";
>
> The location: "jpg.tests.jdk" may be common for all artifacts.
> Suggestion:
>
> public static final String ARTIFACT_BASE = "jpg.tests.jdk";
done in the next commit
> test/lib/jdk/test/lib/security/artifacts/ThirdPartyArtifacts.java line 42:
>
>> 40: // the NSS version
>> 41: public static final String NSS_BUNDLE_VERSION = "3.107";
>> 42: public static final String NSSLIB = "jpg.tests.jdk.nsslib";
>
> It can reuse the common location. also do the bundle vars need to be public ?
>
> Suggestion:
>
> private static final String NSSLIB = ARTIFACT_BASE + ".nsslib";
done in the next commit
> test/lib/jdk/test/lib/security/artifacts/ThirdPartyArtifacts.java line 49:
>
>> 47: revision = NSS_BUNDLE_VERSION,
>> 48: extension = "zip")
>> 49: public static class WINDOWS_X64 {
>
> Please rename the NSS static classes to `NSS_<platform>` as
> ThirdPartyArtifacts will also contain other artifacts for these platforms
> Suggestion:
>
> public static class NSS_WINDOWS_X64 {
done in the next commit
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23988#discussion_r2017135131
PR Review Comment: https://git.openjdk.org/jdk/pull/23988#discussion_r2017135046
PR Review Comment: https://git.openjdk.org/jdk/pull/23988#discussion_r2017135227
PR Review Comment: https://git.openjdk.org/jdk/pull/23988#discussion_r2017134859
