Use codeclimate-parser backend #207
Conversation
codeclimate-hermes
requested review from
wfleming
and removed request for
codeclimate-hermes
codeclimate-hermes
requested review from
gdiggs
and removed request for
codeclimate-hermes
dblandin
requested review from
codeclimate-hermes
and removed request for
gdiggs
codeclimate-hermes
requested review from
maxjacobson
and removed request for
codeclimate-hermes
Dockerfile
Outdated
|
|
||
| RUN bundle install --jobs 4 && \ | ||
| cd /usr/src/app/vendor/php-parser && composer install --no-interaction && \ | ||
| npm install |
There was a problem hiding this comment.
is this npm install-ing inside of the php directory?
There was a problem hiding this comment.
Ahh, good point. I don't think so, but I'm now taking advantage of a composer install flag that sets the working directory, just for the composer dependency installation.
| @@ -0,0 +1,13 @@ | |||
| { | |||
There was a problem hiding this comment.
What's up with this file? Do we want it? If we do, I think we should COPY it in the dockerfile and document how to update it over time
There was a problem hiding this comment.
I'm including it for now because npm install outputs a warning when it writes the file. We'll be removing this particular npm install when we move over to the parser soon, so I'm inclined to punt on the documentation. Added that COPY argument though 👍
Dockerfile
Outdated
| openjdk-8-jdk-headless maven \ | ||
| php php-common php-cli php-fpm php-xml composer \ | ||
| python python2.7 python-pip python-setuptools \ | ||
| ruby2.3 ruby2.3-dev bundler |
There was a problem hiding this comment.
Are some of these things maybe already available via the base image and don't need to be re-installed? I suppose it doesn't hurt to re-install them
There was a problem hiding this comment.
Ahhh, yes, most of these are redundant. Cleaned up!
dblandin
force-pushed
the
devon/parser-backend
branch
3 times, most recently
from
56fc7b9 to
56fa20b
Compare
Dockerfile
Outdated
| RUN cd /usr/src/app/vendor/php-parser/ && composer install --prefer-source --no-interaction | ||
| RUN npm install | ||
| RUN bundle install --jobs 4 --quiet && \ | ||
| composer install --no-interaction --quiet --working-dir /usr/src/app/vendor/php-parser && \ |
There was a problem hiding this comment.
these --quiets are nice too, they definitely make the output less noisy
Dockerfile
Outdated
| @@ -1,73 +1,27 @@ | |||
| FROM ruby:2.3-alpine | |||
| FROM codeclimate/codeclimate-parser:java-beta | |||
There was a problem hiding this comment.
if our channel release strategy is to make the tag based on the branch name, and not the build number, how do we bump the base image?
This looks correct based on our parser release strategy, but I'm just curious how we're planning to handle that...
There was a problem hiding this comment.
That's a good point. We probably shouldn't be using a java-beta tag for the codeclimate-parser image. I'll make some changes there, so we can use a b### tag here as well.
There was a problem hiding this comment.
Updated the java branch of codeclimate-parser to push b###-tagged images that we can use as the base for this channel: https://circleci.com/gh/codeclimate/codeclimate-parser/264
| RUN cd /usr/src/app/vendor/php-parser/ && composer install --prefer-source --no-interaction | ||
| RUN npm install | ||
| RUN bundle install --jobs 4 --quiet && \ | ||
| composer install --no-interaction --quiet --working-dir ./vendor/php-parser && \ |
There was a problem hiding this comment.
I like these --quiets!
This commit updates the Dockerfile to simplify the installation process of languages and dependencies, and also brings in the new codeclimate parser.
dblandin
force-pushed
the
devon/parser-backend
branch
from
56fa20b to
6338b16
Compare
Setup a
javachannel to pull in our new parser backend. This keeps all of the core language implementation intact and unchanged while we build out java support.This PR makes the parser available via the base image. A following PR will add the Java support implementation.