On Mon, 2 Nov 2020 20:21:44 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 46 commits: >> >> - Removing trailing whitespace. >> - Merging master into JDK-8250768. >> - Updating tests after records are a final feature. >> - Fixing tests. >> - Finalizing removal of record preview hooks. >> - Merging master into JDK-8250768 >> - Reflecting review comments. >> - Merge branch 'master' into JDK-8250768 >> - Removing unnecessary cast. >> - Using a more correct way to get URLs. >> - ... and 36 more: >> https://git.openjdk.java.net/jdk/compare/d93e3a7d...2e403900 > > I have read all the files. > > I have added a n umber of various minor non-blocking comments (no need for > re-review( to fix these. But I have a couple of comments/questions before > finally giving approval. > There's a comment in `PreviewListWriter` about annotation members that needs > too be addressed, and I wonder is RECORD and RECORD_COMPONENT need to be > added into PreviewElementKind. Thanks @jonathan-gibbons for your comments! I've tried to update the code based on them, mostly in https://github.com/lahodaj/jdk/commit/743f516c660b577035cdda4510a0bb97937fd9b2 and https://github.com/lahodaj/jdk/commit/e4b02827998fc2e8f19f983aabfb3d720b03d111 A big chunk of the update is generalization of the deprecated and preview list builders and writers into a "summary" list builder and writer. These should also now handle records. For record components, those are a little tricky, as (AFAIK) can't currently have deprecation/preview-ness for them (and hence there is no good way to test any support for record components in these). But the summary build and writer are looking for record components and will fail in case a record component is sent into them. ------------- PR: https://git.openjdk.java.net/jdk/pull/703
