feature: print version on the start of execution and always pull docker image (always update stable) #417
Conversation
pedrocodacy
requested review from
lucassklp,
franciscodua,
machadoit,
jllopes and
a team
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
from
8f0550a to
8413adf
Compare
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
3 times, most recently
from
7f415f9 to
9fa75eb
Compare
pedrocodacy
changed the title
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
2 times, most recently
from
e8fce90 to
45bb1b7
Compare
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
2 times, most recently
from
e1cfdaf to
cd3eb1f
Compare
pedrocodacy
requested review from
machadoit,
pedrobpereira,
rubencodacy and
vascorsd
and removed request for
lucassklp,
franciscodua and
jllopes
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
2 times, most recently
from
b3d7412 to
2e323bf
Compare
pedrocodacy
force-pushed
the
feature/add-version-text-output
branch
from
2e323bf to
a140329
Compare
|
still needs some testing locally |
There was a problem hiding this comment.
The code itself seems ok, it compile and I verified that locally passing: "set codacyAnalysisCli / version := "magic version" and doing compile and then "codacyAnalysisCli / run --version", seems to all be working.
The script itself I already left comments inline.
The only issue I'm not sure is if this will really change the version for the released binary, as is I believe it stays the same, since looking at the circle ci file I only see one single place changing the version with sbt. It has the following at line 178 - sbt "set codacyAnalysisCli / version := \"dev-snapshot\";, so please confirm if this also needs to change.
If that verification comes afterwards or the change itself, we can still approve and go on with the PR as is.
| @@ -31,6 +31,7 @@ run() { | |||
| output_volume="--volume ${OUTPUT_DIRECTORY}:${OUTPUT_DIRECTORY}"; | |||
| fi | |||
| local CODACY_ANALYSIS_CLI_VERSION="${CODACY_ANALYSIS_CLI_VERSION:-stable}" | |||
| docker pull codacy/codacy-analysis-cli:${CODACY_ANALYSIS_CLI_VERSION} | |||
There was a problem hiding this comment.
This seems to work as I tried locally with the changed script to fetch a latest stable version or other version.
I'd reccomened the change that machado said to add quotes, I tried with things like env CODACY_ANALYSIS_CLI_VERSION="stuff stuff" and it really gets a little better UX with the quotes.
Also It seems better to have the commands connected with a &&, I just checked locally and seems to behave better in case of a bad tag passed since it doesn't try to run something invalid after already tried to pull something invalid and failling. So:
| docker pull codacy/codacy-analysis-cli:${CODACY_ANALYSIS_CLI_VERSION} | |
| docker pull codacy/codacy-analysis-cli:"${CODACY_ANALYSIS_CLI_VERSION}" && |
There was a problem hiding this comment.
Also bellow on line 45:
codacy/codacy-analysis-cli:${CODACY_ANALYSIS_CLI_VERSION} should be codacy/codacy-analysis-cli:"${CODACY_ANALYSIS_CLI_VERSION}"
So it's consistent.
|
closing since this is stale, we can consider having these changes later |
did this one on the side while fixing tests in worker
I havent tested it yet, but its ready for review of the approach (actually I tested that the env of the docker contains the version, so further testing should be a quick thing)