feat: get rid of poetry dependency in version checker#6
feat: get rid of poetry dependency in version checker#6OriginalUtkin wants to merge 2 commits intomasterfrom
Conversation
| version_in_master=$(eval "git show origin/master:$version_file | $command") | ||
|
|
||
| if (! is_output_valid $version_in_branch) || (! is_output_valid $version_in_master); then | ||
| echo "Version update is not correct" |
There was a problem hiding this comment.
it may be beneficial to provide what is the version - in case of error it can be something else than what is in master/current branch.
There was a problem hiding this comment.
agree, can be done by moving echo commands ("Version in branch", "Version in master") before that if
| if [[ ("$2" == "poetry") || (("$2" == "") && ( -f pyproject.toml)) ]]; then | ||
| function is_output_valid() { | ||
| line_numbers=$(eval "echo $1 | wc -l") | ||
| is_version_correct=$(eval "echo $1 | grep -E '[0-9.]?'") |
There was a problem hiding this comment.
I'm not sure that 0-9.]? is enough.
As I said, if git something will fail, the version can contain some error message which is easily matched by this 0-9.]? regexp. I suggest ^([0-9]+\.)+[0-9]+$ which is strict and covers all our existing versions.
In case it will not cover some versions, we can update it later according the needs.
| version_in_master=$(eval "git show origin/master:$version_file | $command") | ||
|
|
||
| if (! is_output_valid $version_in_branch) || (! is_output_valid $version_in_master); then | ||
| echo "Version update is not correct" |
There was a problem hiding this comment.
agree, can be done by moving echo commands ("Version in branch", "Version in master") before that if
|
|
||
| if [[ ("$2" == "poetry") || (("$2" == "") && ( -f pyproject.toml)) ]]; then | ||
| function is_output_valid() { | ||
| line_numbers=$(eval "echo $1 | wc -l") |
There was a problem hiding this comment.
probably no need to use eval here
| version_file="pyproject.toml" | ||
| command="poetry version -s" | ||
| elif [[ ("$2" == "version_file") || (("$2" == "") && ( -f VERSION)) ]]; then | ||
| command="sed 's/[\ \"]*//g' | grep -E '(^\[tool\.poetry\])|(^version)' | sed -E 's/\[tool.poetry\]//g' | sed -E 's/version[[:blank:]]?=[[:blank:]]?//g'" |
There was a problem hiding this comment.
please consider using functions instead of string command:
function cmd1() { echo "cmd1" }
function cmd2() { echo "cmd2" }
alias chosen_command='cmd1'
chosen_command # calls cmd1
``There was a problem hiding this comment.
also I think it is possible to pipe output of previous command as the input of the function, please check https://stackoverflow.com/a/11454477/1768976
3 participants
In previous release poetry dependency was introduced in order to be able check version of poetry file. It was done as quick solution just to make it works.
This merge request removes such dependency in order not to relate on it inside the container it is being run in CI/CD container.