<i18n dev> RFR: 8315097: Rename createJavaProcessBuilder [v2]
Roger Riggs
rriggs at openjdk.org
Tue Aug 29 14:09:12 UTC 2023
On Tue, 29 Aug 2023 09:12:51 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:
>> Rename createJavaProcessBuilder so that it is not used by mistake instead of createTestJvm.
>>
>> I have used the following sed script: `find -name "*.java" | xargs -n 1 sed -i -e "s/createJavaProcessBuilder(/createJavaProcessBuilderIgnoreTestJavaOpts(/g"`
>>
>> Then I have manually modified ProcessTools.java. In that file I have moved one version of createJavaProcessBuilder so that it is close to the other version. Then I have added a javadoc comment in bold telling:
>>
>> /**
>> * Create ProcessBuilder using the java launcher from the jdk to
>> * be tested.
>> *
>> * <p><b> Please observe that you likely should use
>> * createTestJvm() instead of this method because createTestJvm()
>> * will add JVM options from "test.vm.opts" and "test.java.opts"
>> * </b> and this method will not do that.
>> *
>> * @param command Arguments to pass to the java command.
>> * @return The ProcessBuilder instance representing the java command.
>> */
>>
>>
>> I have used the name createJavaProcessBuilderIgnoreTestJavaOpts because of the name of Utils.prependTestJavaOpts that adds those VM flags. If you have a better name I could do a rename of the method. I kind of like that it is long and clumsy, that makes it harder to use...
>>
>> I have run tier 1 testing, and I have started more exhaustive testing.
>
> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
>
> copyright
I don't think this is the best change across so many files.
It gives a very ugly name to a common test function and affects a very large number of tests.
-------------
Changes requested by rriggs (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15452#pullrequestreview-1600512718
More information about the i18n-dev
mailing list