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

Upgraded to gradle 6.5.1 + added gradlew scripts #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Upgraded to gradle 6.5.1 + added gradlew scripts #211

wants to merge 1 commit into from

Conversation

ieugen
Copy link

@ieugen ieugen commented Jul 22, 2020

Signed-off-by: Eugen Stan [email protected]

Description

Upgrade gradle and added the gradle scripts.

Motivation and Context

The normal way to build apps locally is to use the ./gradlew script that will get the exact version of gradle needed to build the project.

Adding the script makes it so that users don't require gradle installed locally.
It will be downloaded.

How Has This Been Tested?

Ran ./verify.sh locally.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Version change (see: Impact to existing users)

Impact to existing users

I don't see negative impact.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@alexellis
Copy link
Member

I don't want to see the Gradle folders and scripts in the template, please can you remove those and just bump up the version and make any required changes to the Gradle build files? You said that "compile" is deprecated now?

@ieugen
Copy link
Author

ieugen commented Jul 22, 2020

Having the version files without the wrapper scripts provides little to no value.
They are designed to work together.
The wrapper will use the version files to get the exact version of gradle to build the project.
The gradle wrapper is meant to be added to version control.

Gradle developers expect the wrapper.
I always use the wrapper to build gradle apps.

I know many people avoid adding binaries to source control.
I avoid that too but sometimes it makes sense and it is ok.
This is IMO one of those cases.

https://docs.gradle.org/current/userguide/gradle_wrapper.html#sec:adding_wrapper

To make the Wrapper files available to other developers and execution environments you’ll need to check them into version control. All Wrapper files including the JAR file are very small in size. Adding the JAR file to version control is expected. Some organizations do not allow projects to submit binary files to version control. At the moment there are no alternative options to the approach.

@alexellis
Copy link
Member

I think the java8 template should be deprecated and removed, isn't Java 8 out of support now?

@ieugen
Copy link
Author

ieugen commented Jul 23, 2020

I think it's safe to do that sine openfaas is targeting new implementations and less so legacy apps.
Also, the fact that we build the mode/entrypoint code with Java 11 does not mean the functions can't run on any 11+ Java version.
I would very much like to have the functions available in many java versions.
I think we should allow this as an argument when we build the image ( I need to read more on the documentation to find out what I can and can't do OOTB in openfaas ).

Java is evolving at a steady 6 month pace and writing functions with some of the newer features is a big win IMO.
People can use new features in smaller (safer) code bases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants