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.

Sign up for GitHub

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

Jump to bottom

[WIP] Java 11 upgrade #391

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

[WIP] Java 11 upgrade #391

wants to merge 7 commits into from

Conversation

Tylerwbrown
Copy link
Contributor

Notes about the upgrade

  • ClassLoader can no longer be cast to URLClassLoader. This was never an intended feature in Java apparently, and was just an implementation detail, which is why although Java 9 is described as "backwards compatible", this was a breaking change. Luckily the only place where we used it, it wasn't necessary.
  • We're getting warnings about illegal reflective access now, some important notes:
    • "The first reflective-access operation to any such package causes a warning to be issued, but no warnings are issued after that point."
      This means I don't have a good sense of how widespread this issue is in our codebase, but I know we use reflection quite a bit, so it would make sense that this is a large issue
    • "This warning cannot be suppressed."
    • "All illegal access operations will be denied in a future release"

TODO:

  • Fix remaining test failure (currently only one left)
  • Fix Javadocs (currently disabled)

@Tylerwbrown
Gradle, spotless plugin, and java upgrade.
5d7b2e2
@Tylerwbrown
various library updates
de9763d
@Tylerwbrown
Parameters update
36e0785
@Tylerwbrown
gradle change
ea63491
@Tylerwbrown
Merge branch 'develop' into feature/CON-631
aac7f8f
@Tylerwbrown
Changed way we retrieved the class path, more details below
0013153 
The old way we retrieved the classpath wasn't ever supported by Java, it was an implementation detail on the JVM that got removed in Java 9, even though Java 9 was "backwards compatible", it broke code that wasn't officially supported.
@Tylerwbrown
Changed some details about fork, details below
0ee6162 
- Used local variable type inference
- Made use of final consistent
- JavaApp already had a constructor that takes just the sourcecode and manages getting the classpath
- Used Java 8+ lambda syntax over new Runnable() { ..run } syntax
- Made variables more understandable. They were lacking context before that took a little too long to figure out on the first read, i.e:
    - is input the filepath or the data that'll go in the file?
    - is output the filepath or the data that'll go in the file?
    - is source the filepath or the data that'll go in the file?
the answer was different for each of these but nothing about their names or type signified this, so I changed them. 
Also "ret" isn't a good name.
@Tylerwbrown
Copy link
Contributor Author

Tylerwbrown commented Aug 2, 2019 •
edited
Loading

I don't believe that Javadocs are functional in the current version of Gradle + Java 11.

The issue is detailed in the following spots:

@jtnelson if you could take a look it'd be appreciated, hopefully I'm missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Tylerwbrown