Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8246774: implement Record Classes as a standard feature in Java #290

Closed

Conversation

@vicente-romero-oracle
Copy link
Contributor

@vicente-romero-oracle vicente-romero-oracle commented Sep 21, 2020


Progress

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

Testing

Linux x64 Windows x64 macOS x64
Build ✔️ (5/5 passed) ✔️ (2/2 passed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) (1/9 running) ✔️ (9/9 passed)

Issue

  • JDK-8246774: implement Record Classes as a standard feature in Java

Reviewers

Contributors

  • Vicente Romero <vromero@openjdk.org>
  • Harold Seigel <hseigel@openjdk.org>
  • Chris Hegarty <chegar@openjdk.org>

Download

$ git fetch https://git.openjdk.java.net/jdk pull/290/head:pull/290
$ git checkout pull/290

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Sep 21, 2020

👋 Welcome back vromero! 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 openjdk bot added the rfr label Sep 21, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Sep 21, 2020

@vicente-romero-oracle The following labels will be automatically applied to this pull request: compiler core-libs hotspot-runtime javadoc kulla serviceability.

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

@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Sep 21, 2020

Please review the fix for [1]. The intention of this patch is to make records final removing the need to use --enable-preview in order to be able to include a record declaration in a source or for the VM to execute code compiled from record classes,

Thanks

[1] https://bugs.openjdk.java.net/browse/JDK-8246774

@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 21, 2020

@jddarcy
Copy link
Member

@jddarcy jddarcy commented Sep 21, 2020

/csr

@openjdk openjdk bot added the csr label Sep 21, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Sep 21, 2020

@jddarcy has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@vicente-romero-oracle please create a CSR request and add link to it in JDK-8246774. This pull request cannot be integrated until the CSR request is approved.

@jddarcy
Copy link
Member

@jddarcy jddarcy commented Sep 21, 2020

Hi Vicente,
Please file a separate subtask for the javax.lang.model changes. This helps with the JSR 269 MR paperwork.
Thanks,
-Joe

@vicente-romero-oracle vicente-romero-oracle marked this pull request as draft Sep 21, 2020
@openjdk openjdk bot removed the rfr label Sep 21, 2020
@vicente-romero-oracle vicente-romero-oracle marked this pull request as ready for review Sep 21, 2020
@openjdk openjdk bot added the rfr label Sep 21, 2020
@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Sep 21, 2020

note: I have removed from the original patch the code related to javax.lang.model, I will publish them in a separate PR

@ChrisHegarty
Copy link
Member

@ChrisHegarty ChrisHegarty commented Sep 22, 2020

@vicente-romero-oracle I noticed that we can also remove the preview args from the record serialization tests and ObjectMethodsTest. I opened a PR against the branch in your fork. You should be able to just merge in the changes. See vicente-romero-oracle#1

@openjdk
Copy link

@openjdk openjdk bot commented Sep 23, 2020

⚠️ @vicente-romero-oracle This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 23, 2020

Mailing list message from Vicente Romero on kulla-dev:

good catch Chris, thanks for the patch,

Vicente

On 9/22/20 5:51 AM, Chris Hegarty wrote:

@mlchung
Copy link
Member

@mlchung mlchung commented Sep 24, 2020

What is the policy of @since release value when a preview API becomes final. I would expect @since should be updated from 14 to 16 because 16 is the Java SE release these APIs are added??

@openjdk
Copy link

@openjdk openjdk bot commented Sep 24, 2020

@vicente-romero-oracle this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8246774
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 24, 2020

Mailing list message from Alex Buckley on kulla-dev:

On 9/23/2020 5:26 PM, Mandy Chung wrote:

What is the policy of `@since` release value when a preview API
becomes final. I would expect `@since` should be updated from 14
to 16 because 16 is the Java SE release these APIs are added??

Yes. Per
http://openjdk.java.net/jeps/12#Specifications-of-Preview-Features :

In particular, all elements of a preview API must have the following
tags: ... An @SInCE tag that indicates the release when [the API
element] was first added. *If the API element is eventually made final
and permanent in Java SE $N, then the @SInCE tag must be changed to
indicate the $N release (the element's history prior to $N is not of
long-term interest).*

Alex

@mlchung
Copy link
Member

@mlchung mlchung commented Sep 24, 2020

Thanks Alex.

@vicente-romero-oracle @since needs to be changed.

Copy link

@coleenp coleenp left a comment

The classfile parser changes look good to me.

@openjdk openjdk bot removed the rfr label Oct 2, 2020
@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Oct 2, 2020

/contributor remove briangoetz
/contributor remove jjg
/contributor remove mcimadamore
/contributor remove darcy
/contributor remove jlahoda

@openjdk
Copy link

@openjdk openjdk bot commented Oct 2, 2020

@vicente-romero-oracle
Contributor Brian Goetz <briangoetz@openjdk.org> successfully removed.

@openjdk openjdk bot added the rfr label Oct 2, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 2, 2020

@vicente-romero-oracle
Contributor Jonathan Gibbons <jjg@openjdk.org> successfully removed.

@openjdk
Copy link

@openjdk openjdk bot commented Oct 2, 2020

@vicente-romero-oracle
Contributor Maurizio Cimadamore <mcimadamore@openjdk.org> successfully removed.

@openjdk
Copy link

@openjdk openjdk bot commented Oct 2, 2020

@vicente-romero-oracle
Contributor Joe Darcy <darcy@openjdk.org> successfully removed.

@openjdk
Copy link

@openjdk openjdk bot commented Oct 2, 2020

@vicente-romero-oracle
Contributor Jan Lahoda <jlahoda@openjdk.org> successfully removed.

@vicente-romero-oracle vicente-romero-oracle mentioned this pull request Oct 9, 2020
3 of 3 tasks complete
@vicente-romero-oracle vicente-romero-oracle changed the title 8246774: implementing Record Classes as a standard feature in Java 8246774: implement Record Classes as a standard feature in Java Oct 17, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 18, 2020

@vicente-romero-oracle 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:

8246774: implement Record Classes as a standard feature in Java

Co-authored-by: Vicente Romero <vromero@openjdk.org>
Co-authored-by: Harold Seigel <hseigel@openjdk.org>
Co-authored-by: Chris Hegarty <chegar@openjdk.org>
Reviewed-by: coleenp, jlahoda, sspitsyn, chegar

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added ready and removed merge-conflict labels Oct 18, 2020
@vicente-romero-oracle
Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle commented Oct 18, 2020

/integrate

@openjdk openjdk bot closed this Oct 18, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Oct 18, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Oct 18, 2020

@vicente-romero-oracle Pushed as commit c17d585.

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

@vicente-romero-oracle vicente-romero-oracle deleted the vicente-romero-oracle:JDK-8246774 branch Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment