Upgrade to Gradle 5.0#900
Conversation
Remove errorprone Remove findbugs Remove buildSrc project Move generated code into core Give each project its own buildscript Remove AutoValue, replace with lombok
Revert lombok change
Fix new a problem identified by spotbugs
Codecov Report
@@ Coverage Diff @@
## master #900 +/- ##
============================================
- Coverage 51.71% 50.37% -1.35%
============================================
Files 247 304 +57
Lines 7812 8318 +506
Branches 533 535 +2
============================================
+ Hits 4040 4190 +150
- Misses 3590 3951 +361
+ Partials 182 177 -5 |
AustinShalit
left a comment
There was a problem hiding this comment.
* Where:
Build file '/Users/austinshalit/git/GRIP/build.gradle.kts' line: 50
* What went wrong:
No .git directory found!
|
And yes... there is a git directory. |
| script: | ||
| # Only run code generation tests on OSX -- linux requires sudo to install OpenCV dependencies, and that environment | ||
| # will not be able to run MainWindowTest.testDragOperationFromPaletteToPipeline | ||
| - ./gradlew check jacocoTestReport jacocoRootReport --stacktrace -Pheadless=true -PlogTests --scan; |
There was a problem hiding this comment.
Why the removal of the --scan?
There was a problem hiding this comment.
It was causing Travis to fail from having to accept the license agreement. Since Travis is getting removed anyway, I'm not worried about temporarily reducing its workload to keep the important stuff working until then
| repositories { | ||
| jcenter() | ||
| maven { | ||
| setUrl("https://github.com/WPIRoboticsProjects/opencv-maven/raw/mvn-repo") |
There was a problem hiding this comment.
HTTPS support. Wonderful!
Github might get pissy though if we keep using them as a maven repo.
build.gradle.kts
Outdated
| @@ -0,0 +1,201 @@ | |||
| import org.ajoberstar.grgit.Grgit | |||
| import com.github.spotbugs.* | |||
There was a problem hiding this comment.
Can we avoid wildcard imports?
| tasks.withType<Wrapper>().configureEach { | ||
| gradleVersion = "5.0" | ||
| distributionType = Wrapper.DistributionType.ALL | ||
| } |
There was a problem hiding this comment.
I usually keep this at the bottom. Why 5.0 instead of 5.1? The memory leak issue?
| plugin("org.ajoberstar.grgit") | ||
| } | ||
| grgit = Grgit.open() | ||
| } |
There was a problem hiding this comment.
You could do this as:
val grgit: Grgit? by lazy {
// blah....
}| jcenter() | ||
| maven { | ||
| name = "WPILib Maven Release" | ||
| setUrl("http://first.wpi.edu/FRC/roborio/maven/release") |
There was a problem hiding this comment.
Booo. Not a fan of http here.
Can you use the new Gradle repository filers so that only artifacts that are actually on these servers get searched in these repos?
There was a problem hiding this comment.
first.wpi.edu unfortunately does not have an HTTPS cert
The repository filter looks nice, though
There was a problem hiding this comment.
Darn, the filtering isn't available in 5.0
There was a problem hiding this comment.
Is there any sort of hash validation we can do before we just start arbitrarily running code that was downloaded insecurely?
Is there some way we can validate the SHA 256 hash of it first or something?
There was a problem hiding this comment.
I don't think there's anything we can do beyond what I assume Gradle does already with using the .md5 or .sha hash files in the Maven repo.
This is also out of scope for this PR
| reports { | ||
| xml.isEnabled = false | ||
| emacs.isEnabled = true | ||
| } |
There was a problem hiding this comment.
You can get spotbugs to produce an html report if we don't want to deal with the emacs one anymore.
| } | ||
|
|
||
| tasks.withType<Javadoc> { | ||
| source(tasks.named<JavaCompile>("compileJava").map { it.source }) |
| additionalSourceDirs(srcFiles) | ||
| sourceDirectories.from(srcFiles) | ||
| classDirectories.from(files(sourceSets["main"].output)) | ||
| executionData.from(tasks.named<JacocoReport>("jacocoTestReport").map { it.executionData }) |
There was a problem hiding this comment.
Are these froms cumulative or last one wins?
This is the logic I've been using for creating a root report recently.
There was a problem hiding this comment.
I have no idea. This is just a port of the old task (which wasn't ever running AFAICT) to Kotlin.
| } | ||
| assert project.buildFile.isFile(): "File named $groovyName or $kotlinName must exist." | ||
| project.children.each { setUpChildProject(it) } | ||
| } |
There was a problem hiding this comment.
This file can be Kotlin based now if you want it to be. Up to you though.
| tasks.named<JavaExec>("run") { | ||
| classpath = sourceSets["main"].runtimeClasspath | ||
| main = application.mainClassName | ||
| args = listOf("windowed") |
There was a problem hiding this comment.
For launching the preloader by itself. See the preloader application class
| compile(group = "com.hierynomus", name = "sshj", version = "0.16.0") | ||
| compile(group = "org.apache.velocity", name = "velocity", version = "1.7") | ||
|
|
||
| val coreTestOutput = project(":core").dependencyProject.sourceSets["test"].output |
There was a problem hiding this comment.
Eventually, we should fix this better.
JLLeitschuh
left a comment
There was a problem hiding this comment.
Overall really nice work. Just a few comments on things to potentially clean up.
:coreSupersedes and closes #860