Update scalastyle, switch to Scala-based parsing, adopt new Engines requirements #8
Conversation
Chang base image to openjdk:alpine Update ScalaStyle from 0.6.0 -> 1.0.0
Update things
…rt custom message when printing the issue.
major refactor
… used in this case.
Add Dockerfile
|
@chrishulton can you give a heads up when you'll be able to review this PR? |
| }, | ||
| "languages" : ["Scala"], | ||
| "version": "0.1.0", | ||
| "spec_version": "0.0.1" |
There was a problem hiding this comment.
Is the engine compatible with the most recent version of the spec? 0.3.1?
There was a problem hiding this comment.
I think it is. The value 0.0.1 is in the SPEC.md file thus I've used it as an example.
| @@ -0,0 +1,15 @@ | |||
| { | |||
There was a problem hiding this comment.
The spec expects this file at /engine.json. Could you add a COPY to the dockerfile?
COPY engine.json /| ### Building release docker image | ||
| 1. Run `sbt assembly && cp target/scala-2.12/codeclimate-scalastyle-assembly-<version>.jar ./`. | ||
| This will create assembled jar with all dependencies. | ||
| 2. Run `docker build -t codeclimate/codeclimate-scalastyle .` |
There was a problem hiding this comment.
This is fine for now, but it would be great to have this step integrated within the docker image build process, maybe via a multi-stage Dockerfile: https://docs.docker.com/engine/userguide/eng-image/multistage-build
There was a problem hiding this comment.
As I understand SBT build tool is not supported thus this is the only workaround I could think of.
There was a problem hiding this comment.
There are quite a few SBT images out there. Docker definitely can run it. Or have I misunderstood your words?
There was a problem hiding this comment.
Oh, I get what you are saying. Basically use a docker image with sbt to build and assembly package and copy it into the final image. I'll do that.
| @@ -0,0 +1,3 @@ | |||
| target | |||
| .idea | |||
|
|
|||
There was a problem hiding this comment.
This repo might benefit from a .dockerignore file as well.
I don't think the build.sbt or codeclimate-scalastyle-assembly-0.1.0.jar files need to be within the final image.
There was a problem hiding this comment.
codeclimate-scalastyle-assembly-0.1.0.jar is required - it has all the logic for the engine. It is copied as "/usr/src/app/engine-core.jar".
Only files in "src/main/resources/docker" and this jar file are included in the docker image.
There was a problem hiding this comment.
I think once it's COPY'd in, as /usr/src/app/engine-core.jar you can leave the original jar out of the final image.
So if a .dockerignore file included codeclimate-scalastyle-assembly-0.1.0.jar, that would remove the duplicate file, right?
There was a problem hiding this comment.
Dockerfile only copies these files:
COPY codeclimate-scalastyle-assembly-0.1.0.jar /usr/src/app/engine-core.jar
COPY src/main/resources/docker /usr/src/app
COPY src/main/resources/docker/engine.json /
Thus codeclimate-scalastyle-assembly-0.1.0.jar is included only once in the docker image.
Adding it to .dockerignore will break the build process because Docker wouldn't be able to find this jar.
There was a problem hiding this comment.
Ahh, I see. You're right. Thought there was a broader COPY further down, but after looking again, there isn't. LGTM.
.codeclimate.yml
Outdated
| @@ -1,3 +1,9 @@ | |||
| engines: | |||
| scalastyle: | |||
| enabled: true | |||
| config: | |||
| config: src/main/resources/scalastyle_project.xml | |||
| include_paths: | |||
There was a problem hiding this comment.
include_paths isn't normally provided by the user in this way. Code Climate will compute the include_paths, which represent the valid workspace for an engine, based on the exclude_paths at the top level of a Code Climate configuration file. The include_paths are provided within /config.json during an engine run: https://github.com/codeclimate/spec/blob/master/SPEC.md#which-files-to-analyze
There was a problem hiding this comment.
Good to know. I've implemented as you've suggested.
|
Going to go ahead and merge this in for release to a beta channel. @jsippel @ivanobulo Thanks! Nice work! |
No description provided.