Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
On Fri, 10 Jun 2022 18:19:42 GMT, Thiago Henrique Hüpner wrote: >> test/jdk/tools/jar/modularJar/Basic.java line 44: >> >>> 42: >>> 43: import jdk.internal.module.ModuleReferenceImpl; >>> 44: import jdk.internal.module.ModuleResolution; >> >> At some point we need to put in test infrastructure to avoid this and a few >> other tests from depending on JDK internals. > > Do you think it is better to parse the output of `javap` instead of using the > internals to detect if it worked? Parsing the output of javap would be fragile. Instead, I think we'll just add some test infrastructure at some point, maybe when this class file attribute is documented somewhere. - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
On Fri, 10 Jun 2022 18:01:24 GMT, Alan Bateman wrote: >> Thiago Henrique Hüpner has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix copyright year > > test/jdk/tools/jar/modularJar/Basic.java line 44: > >> 42: >> 43: import jdk.internal.module.ModuleReferenceImpl; >> 44: import jdk.internal.module.ModuleResolution; > > At some point we need to put in test infrastructure to avoid this and a few > other tests from depending on JDK internals. Do you think it is better to parse the output of `javap` instead of using the internals to detect if it worked? - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v9]
On Fri, 10 Jun 2022 18:25:09 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Rename dataprovider Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v9]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Rename dataprovider - Changes: - all: https://git.openjdk.org/jdk/pull/9049/files - new: https://git.openjdk.org/jdk/pull/9049/files/962f0ec1..d566ebca Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=07-08 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
On Fri, 10 Jun 2022 16:30:57 GMT, Thiago Henrique Hüpner
wrote:
>> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved
>> flags is used
>
> Thiago Henrique Hüpner has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix copyright year
test/jdk/tools/jar/modularJar/Basic.java line 44:
> 42:
> 43: import jdk.internal.module.ModuleReferenceImpl;
> 44: import jdk.internal.module.ModuleResolution;
At some point we need to put in test infrastructure to avoid this and a few
other tests from depending on JDK internals.
test/jdk/tools/jar/modularJar/Basic.java line 951:
> 949:
> 950: @DataProvider(name = "resolutionNames")
> 951: public Object[][] resolutionNames() {
This is a data provider for resolution warnings, maybe it should be renamed.
-
PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v8]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/9049/files - new: https://git.openjdk.org/jdk/pull/9049/files/8544abd9..962f0ec1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=06-07 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.org/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v7]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Update author - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/7362da92..8544abd9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=05-06 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
On Wed, 8 Jun 2022 14:51:59 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Fix test message Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v6]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Simplify test names - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/c88bc754..7362da92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=04-05 Stats: 30 lines in 1 file changed: 10 ins; 3 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
On Wed, 8 Jun 2022 14:51:59 GMT, Thiago Henrique Hüpner
wrote:
>> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved
>> flags is used
>
> Thiago Henrique Hüpner has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix test message
test/jdk/tools/jar/modularJar/Basic.java line 1050:
> 1048: */
> 1049: @Test(dataProvider = "resolutionNames")
> 1050: public void
> updateFooModuleResolutionDoNotResolveByDefaultAndWarnIfResolved(String
> resolutionName, Predicate hasWarning) throws IOException {
Update the existing tests is good but overly long lines make it too hard to
read (it will make future side-by-side view impossible). Can you trim all these
lines down to something closer to the existing tests in this file?
-
PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix test message - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/c4a719e3..c88bc754 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=03-04 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v4]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Add tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/2acbedb1..c4a719e3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=02-03 Stats: 129 lines in 1 file changed: 129 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v3]
On Tue, 7 Jun 2022 13:30:50 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. The pull request > contains one new commit since the last revision: > > Fix copyright year @Thihup Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v3]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: Fix copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/544ee0f7..2acbedb1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=01-02 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v2]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/cd25d8ad..544ee0f7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used
On Tue, 7 Jun 2022 00:45:17 GMT, Thiago Henrique Hüpner wrote: > 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used The change looks okay but I think we should add a test. You'll have to check the test tree to see if there are existing tests for incubator modules packages as JAR files, there may not be as it's an unusual setup. - PR: https://git.openjdk.java.net/jdk/pull/9049
