Java Tender Loving Care#3
Java Tender Loving Care#3ieugen wants to merge 2 commits intoopenfaas:masterfrom ieugen:java-tender-loving-care
Conversation
|
Thank you for this PR, the Java template could use some community TLC :-) |
|
There are so many changes here and not all of them are going to suit every developer, so I would recommend we agree on a few of them, and get those in via separate PRs. Remember that changes need to be consistent between this project and the openfaas/templates project. What command are you running to generate your Gradle helpers? |
|
I've added the gradlew helpers and updated their versions in both repos. The project.version change you made here would be good to get in, if you are happy to submit that separately? |
* General cleanup and project setup * Using a multi-project build instead of independent builds * Added gradle wrapper scripts. Fixes openfaas/templates#154 * Upgraded gradle version to 6.5.1. Fixes openfaas/templates#211 * Using spotless plugin to format and check source formatting as part of build * Using spotless plugin to add & check license headers in files * Used spotless plugin to reformat the files * Cleaned up and upgraded publishing. Using maven-publish plugin * Documented changes in README * Using java 11 to compile * Scoped sign* properties to openfaas to avoid conflicts Closes #1 . Signed-off-by: Eugen Stan <eugen.stan@netdava.com>
* Using Java 8 for compilation
ieugen
left a comment
ieugen
left a comment
There was a problem hiding this comment.
@alexellis I've added detailed explanations on each change step by step.
Please go through them and if you need more information I will provide them.
I do believe this PR adds a lot of good changes to the project.
I am open (encourage) to invite other java developers for review.
I am open to work with you and other people to solve reasonable change requests.
I'm willing to contribute to the project more once this PR is taken care.
I do not want my contributions to never be merged and end up irrelevant .
Looking forward for feedback,
Eugen
| buildscript { | ||
| group = 'com.openfaas' | ||
| version = '0.1.0' | ||
| } |
There was a problem hiding this comment.
@alexellis : We don't need to setup these inside a buildscript.
One way of configuring common options is to use allprojects and subprojects .
| plugins { | ||
| id 'java' | ||
| id 'application' | ||
| id 'maven' |
There was a problem hiding this comment.
@alexellis : maven plugin is deprecated an being removed. maven-publish is the right way to go.
https://docs.gradle.org/current/userguide/maven_plugin.html#header
| } | ||
|
|
||
| task javadocJar(type: Jar) { | ||
| classifier = 'javadoc' |
There was a problem hiding this comment.
This way of adding a javadoc and sources jars are also deprecated .
Using an IDE with intellisense will notify you that the classifier is deprecated.
https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:complete_example
| mainClassName = 'App' | ||
|
|
||
| dependencies { | ||
| compile 'com.google.guava:guava:23.0' |
There was a problem hiding this comment.
Guava is not used in any of the code here.
Why is it a dependency of the project?
I removed it.
Also, compile is deprecated in favor of other configurations:
https://docs.gradle.org/current/userguide/java_plugin.html#tab:configurations
compile(Deprecated)
Compile time dependencies. Superseded by implementation.
|
|
||
| dependencies { | ||
| compile 'com.google.guava:guava:23.0' | ||
| testCompile 'junit:junit:4.12' |
There was a problem hiding this comment.
junit 5 is better and since we don't have legacy tests why not use it.
https://junit.org/junit5/
| package com.openfaas.entrypoint; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
There was a problem hiding this comment.
This test is being contributted by #4 .
See the diff.
| } | ||
| } | ||
| javadoc { | ||
| if(JavaVersion.current().isJava9Compatible()) { |
There was a problem hiding this comment.
This enabled html5 for JavaDoc.
The old one was html 4 I think.
It got enhanced in java 9 + .
| @@ -1,6 +1,5 @@ | |||
| // Copyright (c) OpenFaaS Author(s) 2020. All rights reserved. | |||
| // Copyright (c) OpenFaaS Author(s) 2018. All rights reserved. | |||
There was a problem hiding this comment.
Spotless has a function to update the copyright year automatically to the current year.
It's not configured yet. It's one line.
https://github.com/diffplug/spotless/tree/main/plugin-gradle#license-header
| package com.openfaas.model; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNull; |
There was a problem hiding this comment.
Migrated to junit 5.
Moved to a package (spotless does not like classes without a package).
No other changes.
| @@ -0,0 +1,4 @@ | |||
| rootProject.name = 'openfaas-java' | |||
|
|
|||
2 participants
Description
Closes #1 .
Signed-off-by: Eugen Stan eugen.stan@netdava.com
Motivation and Context
Java templates needed some care.
After some discussions with @alexellis on Slack I've decided to help out.
Which issue(s) this PR fixes
Provided up
How Has This Been Tested?
./gradlew clean buildbuilds the project.The jars, source and javadoc jars are available under build/libs and have the
Types of changes
Impact to existing users
This should not impact users since the changes are only for the build part of the libraries.
No code delivered to users has been changed by this code.
Integration tests should follow.
Checklist:
git commit -s