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

Update jdk to openjdk8 #225

Closed
regisd opened this issue Nov 1, 2017 · 6 comments
Closed

Update jdk to openjdk8 #225

regisd opened this issue Nov 1, 2017 · 6 comments
Labels
code quality Code health and clean-up task Process tasks
Milestone
1.7.0

Comments

@regisd
Copy link
Member

regisd commented Nov 1, 2017 •
edited
Loading

JFlex is currently compiled with openjdk7.

More and more Maven plugins are compiled with Java 8, and we cannot use them as a result.

Consider moving to openjdk8
@regisd regisd added code quality Code health and clean-up task Process tasks labels Nov 1, 2017
@regisd
Copy link
Member Author

regisd commented Nov 1, 2017

This doesn't imply upgrading ${jflex.jdk.version} AFAIK

@regisd regisd changed the title Update build to java 8 Update jdk to openjdk8 Nov 1, 2017
@regisd
Copy link
Member Author

regisd commented Nov 1, 2017

Note this would make #186 infeasible.

@regisd
Copy link
Member Author

regisd commented Nov 1, 2017

And there was a problem with java 8 and unicode. See #209

@regisd regisd unassigned lsf37 Nov 1, 2017
@regisd regisd added this to the release 1.8.0 milestone Nov 1, 2017
regisd added a commit that referenced this issue Nov 1, 2017
@regisd
Add dep on google-java-format
2b92231 
Let the fmt-maven-plugin reformat the code everytime it is compiled.
Note that version 1.4.0 or above are compiled with java 8, and JFlex allow uses jdk7 (see #225).
@lsf37 lsf37 mentioned this issue Nov 2, 2017
Closed
@lsf37
Copy link
Member

lsf37 commented Nov 2, 2017 •
edited
Loading

It'd be nice if we could provide support for older jdks as long as that is feasible. If our dependencies force us to move up the minimum requirements, then there is not much we can do, of course.

I just checked in #186 which versions are currently around in Travis, and it's basically only openjdk7 and openjdk8. We should make sure things work on openjdk8 for the next release. It sounds like #209 is solved now, so if I'm interpreting things correctly, there is only the change in isJavaIdentifier in Java 8 that messes things up, and I'd argue that the new test output is correct, i.e. it should change together with the jdk version.

This means we should make the test suite aware of the jdk version and allow different outputs depending on jdk, or run it conditionally based on jdk, or something in that vein.

@lsf37 lsf37 mentioned this issue Nov 3, 2017
Merged
@lsf37
Copy link
Member

lsf37 commented Nov 3, 2017

We now have a working test for both (jdk7 and jdk8).

I think this satisfies this issue as well: we can drop jdk7 when our dependencies stop working, and we should make sure jdk8 remains functional.

@lsf37 lsf37 closed this as completed Nov 3, 2017
@lsf37
Copy link
Member

lsf37 commented Nov 3, 2017

( @regisd feel free to re-open if you disagree, of course )

@regisd regisd mentioned this issue Nov 3, 2017
Merged
regisd added a commit that referenced this issue Nov 4, 2017
@regisd
Format java code automatically with google-java-format (#226)
a507242 
* Add build dep on **fmt-maven-plugin** which uses [google-java-format](https://github.com/google/google-java-format).
  - Let the fmt-maven-plugin reformat the code every time it is compiled.
  - Note that version 1.4.0 or above are compiled with java 8, and JFlex allow uses jdk7 (see #225).
  - Use version 1.3.0 for now.

* Move **java_cup** in its own maven module.
  We don't want to reformat java_cup. Move it in its own Maven module and disable

* Use **maven-shade-plugin** to package **jflex.jar** like before.

* Add **Maven wrapper**.
  - Fucking Maven 3.5.0 has a bug which causes maven-shade-plugin to fail with
    `/target/classes (Is a directory)`
  - Import wrapper with `mvn -N io.takari:maven:wrapper`
  - Force add the wrapper jar (because jar extension is in `.gitignore`)
  - Update `run-tests` script with Maven (Travis detects its presence automatically and calls `mvnw` accordingly)
  - The wrapper is configured to use **Maven 3.5.2**

* Don't touch java source for now

* Update ant `buld.xml` as well
@lsf37 lsf37 modified the milestones: 1.8.0, 1.7.0 Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
code quality Code health and clean-up task Process tasks
Projects
None yet
Milestone
1.7.0
Development

No branches or pull requests
2 participants
@regisd @lsf37