JDK-8263545: Convert jpackage to use Stream.toList() #2997

igraves · 2021-03-14T18:22:50Z

This converts jpackage to use Stream.toList() instead of Stream.collect(Collectors.toList()). One piece of code was modified to not mutate a list in addition to one test that used a mutating sort on a list. The rest of the changes are simple substitutions.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8263545: Convert jpackage to use Stream.toList()

Reviewers

Alexey Semenyuk (@alexeysemenyukoracle - Reviewer)
Alexander Matveev (@sashamatveev - Committer) ⚠️ Review applies to ce11489b1a134f727191a68b71456cfa61a2d231

Download

To checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/2997/head:pull/2997
$ git checkout pull/2997

To update a local copy of the PR:
$ git checkout pull/2997
$ git pull https://git.openjdk.java.net/jdk pull/2997/head

bridgekeeper · 2021-03-14T18:23:51Z

👋 Welcome back igraves! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-03-14T18:24:55Z

@igraves The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-03-14T18:28:19Z

Webrevs

02: Full (37d3692)
01: Full - Incremental (ce11489b1a134f727191a68b71456cfa61a2d231)
00: Full (1bb46b5496f682efc9bd9e0cbd4f487db657eb96)

alexeysemenyukoracle · 2021-03-15T17:29:46Z

src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WinMsiBundler.java

Why this explicit cast is needed here?

The explicit cast is needed because Stream.toList() is invariant, but the stream is producing a covariant type (by way of the type inferencer and Stream.of(..) accepting two subclasses of WixFragmentBuilder). We need to introduce a hint to bring the type back to the invariant form. On second thought I'm going to make this explicit type argument instead so it's clear that the casting isn't part to what the code is otherwise doing.

alexeysemenyukoracle · 2021-03-15T17:31:10Z

src/jdk.jpackage/share/classes/jdk/jpackage/internal/DottedVersion.java

I guess this can be simplified down to components.stream().toArray(BigInteger[]::new);

Good catch!

alexeysemenyukoracle

Looks good. Minor improvements suggested.

openjdk · 2021-03-15T19:26:34Z

@igraves This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8263545: Convert jpackage to use Stream.toList()

Reviewed-by: asemenyuk, almatvee

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 89 new commits pushed to the master branch:

b49c589: 8263658: Use the blessed modifier order in java.base
1572f3c: 8263852: Unused method SoftRefPolicy::use_should_clear_all_soft_refs
57497ab: 8263821: Remove unused MethodTypeForm canonicalization codes
4d51a82: 8263818: Release JNI local references in get/set-InetXXAddress-member helper functions of net_util.c
701fd9d: 8262476: Add filter to speed up CompileCommand lookup
454af87: 8263185: Mallinfo deprecated in glibc 2.33
d24e4cf: 8263481: Specification of JComponent::setDefaultLocale doesn't mention that passing 'null' restores VM's default locale
1a21f77: 8263482: Make access to the ICC color profiles data multithread-friendly
d185655: 8263832: Shenandoah: Fixing parallel thread iteration in final mark task
434a399: 8260274: Cipher.init(int, key) does not use highest priority provider for random bytes
... and 79 more: https://git.openjdk.java.net/jdk/compare/d6b5e1809bbeb31ec87a5a73f0f49de41c6fa0f7...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@alexeysemenyukoracle, @sashamatveev) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

igraves · 2021-03-15T19:27:10Z

/integrate

openjdk · 2021-03-15T19:27:51Z

@igraves
Your change (at version ce11489b1a134f727191a68b71456cfa61a2d231) is now ready to be sponsored by a Committer.

kevinrushforth · 2021-03-18T19:43:10Z

@igraves as a "best practice" try to avoid doing a force-push to a branch with an active code review. It makes it hard for reviewers to see what has changed. If you need to merge changes from master, use "git merge master" instead.

@alexeysemenyukoracle @sashamatveev Can you re-review this so Ian can integrate it?

igraves · 2021-03-18T19:51:06Z

@kevinrushforth Understood, thanks!

sashamatveev · 2021-03-18T22:00:04Z

Looks fine.

igraves · 2021-03-19T17:19:13Z

/integrate

igraves · 2021-03-19T17:19:30Z

Ready for sponsoring when you all are.

openjdk · 2021-03-19T17:20:06Z

@igraves
Your change (at version 37d3692) is now ready to be sponsored by a Committer.

alexeysemenyukoracle · 2021-03-19T19:50:23Z

/sponsor

openjdk · 2021-03-19T19:51:47Z

@alexeysemenyukoracle @igraves Since your change was applied there have been 92 commits pushed to the master branch:

ed701ea: 8262271: SA: Add new stress test that tests getting the stack trace of an active thread
30da9bc: 8263546: Add "findsym" command to clhsdb.html help file
d41f751: 8247370: Clean up unused printing code in awt_PrintJob.cpp
b49c589: 8263658: Use the blessed modifier order in java.base
1572f3c: 8263852: Unused method SoftRefPolicy::use_should_clear_all_soft_refs
57497ab: 8263821: Remove unused MethodTypeForm canonicalization codes
4d51a82: 8263818: Release JNI local references in get/set-InetXXAddress-member helper functions of net_util.c
701fd9d: 8262476: Add filter to speed up CompileCommand lookup
454af87: 8263185: Mallinfo deprecated in glibc 2.33
d24e4cf: 8263481: Specification of JComponent::setDefaultLocale doesn't mention that passing 'null' restores VM's default locale
... and 82 more: https://git.openjdk.java.net/jdk/compare/d6b5e1809bbeb31ec87a5a73f0f49de41c6fa0f7...master

Your commit was automatically rebased without conflicts.

Pushed as commit 0b5216a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

openjdk bot added the rfr Pull request is ready for review label Mar 14, 2021

openjdk bot added the core-libs core-libs-dev@openjdk.org label Mar 14, 2021

alexeysemenyukoracle reviewed Mar 15, 2021

View reviewed changes

alexeysemenyukoracle approved these changes Mar 15, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 15, 2021

openjdk bot added the sponsor Pull request is ready to be sponsored label Mar 15, 2021

sashamatveev approved these changes Mar 15, 2021

View reviewed changes

igraves added 2 commits March 15, 2021 23:51

Converting jpackage to use Stream.toList()

846eb9c

Clarifying type argument and shorting a toArray invocation

37d3692

igraves force-pushed the convert-jpackage-stream-tolist branch from ce11489 to 37d3692 Compare March 16, 2021 04:51

openjdk bot removed the sponsor Pull request is ready to be sponsored label Mar 16, 2021

alexeysemenyukoracle approved these changes Mar 19, 2021

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Mar 19, 2021

openjdk bot closed this Mar 19, 2021

openjdk bot added the integrated Pull request has been integrated label Mar 19, 2021

openjdk bot removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 19, 2021

JDK-8263545: Convert jpackage to use Stream.toList() #2997

JDK-8263545: Convert jpackage to use Stream.toList() #2997

Uh oh!

Conversation

igraves commented Mar 14, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Download

Uh oh!

bridgekeeper bot commented Mar 14, 2021

Uh oh!

openjdk bot commented Mar 14, 2021

Uh oh!

mlbridge bot commented Mar 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

alexeysemenyukoracle Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

igraves Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

igraves Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igraves commented Mar 15, 2021

Uh oh!

openjdk bot commented Mar 15, 2021

Uh oh!

kevinrushforth commented Mar 18, 2021

Uh oh!

igraves commented Mar 18, 2021

Uh oh!

sashamatveev commented Mar 18, 2021

Uh oh!

igraves commented Mar 19, 2021

Uh oh!

igraves commented Mar 19, 2021

Uh oh!

openjdk bot commented Mar 19, 2021

Uh oh!

alexeysemenyukoracle commented Mar 19, 2021

Uh oh!

openjdk bot commented Mar 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

igraves commented Mar 14, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 14, 2021 •

edited

Loading

openjdk bot commented Mar 15, 2021 •

edited

Loading