pygradle icon indicating copy to clipboard operation
pygradle copied to clipboard

Convert codebase to Java 7

Open ethankhall opened this issue 9 years ago • 11 comments

In an effort to make sure that the code is stable and doesn't get small bugs, lets try to move most (not all) of the code into Java. We should also adapt the code to match java8 style.

Some things to consider: [ ] - Using Files and Paths vs File [ ] - Using try with resource [ ] - Doing multi catches

ethankhall avatar Aug 17 '16 23:08 ethankhall

I could help with this a bit while I learn the plugin. I currently have two questions regarding this work:

  1. Do we care about backward compatibility regarding the ABI?
  2. Would you consider this work as incremental PRs (like one or more class at a time)?

Shad0w1nk avatar Aug 23 '16 20:08 Shad0w1nk

By ABI to you mean the JNI?

Yep! If you want to make small changes open a review and we'll merge them as they come in, no reason to hold onto incremental updates :-)

ethankhall avatar Aug 23 '16 23:08 ethankhall

I read this in the Gradle 3.0 release note [1](see section). There is ways to work around if compatibility is required. We could use the same workaround as Gradle use and once the conversion is finished, we can remove the fix in the next major release. It real depends on how you feel about honoring the compatibility between version (if it's a strict semantic versioning [2]).

[1] https://docs.gradle.org/3.0/release-notes [2] http://semver.org/

Shad0w1nk avatar Aug 24 '16 00:08 Shad0w1nk

I don't think it will be a big deal. I've been converting classes for a while to Java and haven't ran into the issue. We are also on 0.3 so this is the time to do these changes.

ethankhall avatar Aug 24 '16 00:08 ethankhall

Sounds like a good plan.

Shad0w1nk avatar Aug 24 '16 00:08 Shad0w1nk

My 2 cents on forcing Java 8 on plugin users; Please consider supporting the same Java versions as Gradle, without requiring workarounds. This means, if you are supporting Gradle 2.x, then support Java 6 and up. If you only intend on supporting Gradle 3+, then support Java 7 and up.

Personally I think Netflix made a mistake in dropping support for running their plugins in Java 6. IMO, it is much more community-friendly if you support the same Java versions as Gradle does. For us, Netflix's decision has caused nothing but pain, blocking us from upgrading their plugins and, even worse, from upgrading Gradle due to some of their plugin's usage of Gradle internals. Using workarounds to support Java 6 may seem easy, but we've found that quite challenging at scale, despite our efforts in using "enterprise Gradle" techniques.

oesolutions avatar Aug 24 '16 20:08 oesolutions

That is actually a very good and valid point. At the current state, there isn't any dependency on the software model so the target could be Java 6 and up with Gradle 2.x. With the software model (issue 30), I would suggest riding the latest wave, aka. Gradle 3+ and Java 7. Although the plugin is quite new and could just hop directly on Gradle 3+. It just depends how many people wants to jump on board and use the plugin.

Shad0w1nk avatar Aug 24 '16 22:08 Shad0w1nk

I would be OK with dropping it down to Java 7 support, I don't want to go back to Java 6 (not receiving support since Feb 2013) since we do a lot of file operations, it would be a pain to have to stop using NIO and would probably introduce more external dependencies. If you look at Gradle's source they do a lot of hoop-jumping to be able to use NIO when they can and it's really a pain.

We could update the docs and say we recommend using Gradle 3, but will be back compatible to 2.7 (random version) as long as you use Java 7.

I think (haven't verified) that we are using some Java 8 features right now and they would have to be down-converted.

ethankhall avatar Aug 24 '16 23:08 ethankhall

Sounds like a good compromise to me. What do you think @oesolutions?

Shad0w1nk avatar Aug 25 '16 00:08 Shad0w1nk

I agree, that sounds fair enough. Covers my concerns for at least Gradle 3 and should help support the widest user base.

oesolutions avatar Aug 25 '16 05:08 oesolutions

Update: the code base is now Java 7 compliant.

ethankhall avatar Sep 03 '16 02:09 ethankhall