Remove maven and synchronise release workflow #41
Conversation
pyproject.toml
Outdated
| packages = [{include = "nuvla"}] | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = "^3.8" |
There was a problem hiding this comment.
@ignacio-penas Should it not be 3.11?
There was a problem hiding this comment.
The ^ operator indicates that we support any python 3 version whose minor is >=8
There was a problem hiding this comment.
Changed to: ">=3.9, <3.12"
.github/workflows/unittest.yml
Outdated
|
|
||
| # Temporary disabled until we solve the issue with the sonarcloud token | ||
| - name: SonarCloud Scan | ||
| if: matrix.python_version == '3.11' |
There was a problem hiding this comment.
@ignacio-penas If possible I would define the following global env vars
PYTHON_VER_MATRIX=["3.9", "3.10", "3.11"]
PYTHON_VER_LAST=$(last ${PYTHON_VER_MATRIX}) # I'm obviously hand-waving here
and then using it in the matrix definition list
python_version: ${PYTHON_VER_MATRIX}
and then use ${PYTHON_VER_LAST} here and a bit below.
There was a problem hiding this comment.
@konstan github workflows does not allow global env vars (Only if defined from the repository it self) and when defining envs within a job, the allowed types are string, float and int. So I think this would overcomplicate things.
So I'd say we either do it with repository (or organization) level variables. Or leave it as is.
For the first approach, this is the implementation done to build the matrix on the validation workflows:
https://github.com/nuvlaedge/nuvlaedge/blob/c24f77632cfc1f7480719ca908e92ed61fcf9f2c/.github/workflows/validation.yml#L43-L63
There was a problem hiding this comment.
This was my suspicion when I was commenting. What I don't like is "3.11" is all over the "place". So, I tried to suggest a refactor. But if, as you say, the vector definitions are not possible, I would still opt for defining the last (highest supported) version number as I see it's used in a number of places.
There was a problem hiding this comment.
Solved using the repository level variable: SUPPORTED_PYTHON_VERSIONS. Then, using jq to extract the last value of the array.
So now updating supported python version versions would require updating the above mentioned variable and pyproject.toml file.
| on: | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
@ignacio-penas Is this for future possible change?
There was a problem hiding this comment.
@konstan I don't think I understand your question.
The trigger for the release action should be done only when merging into main so that the release PR is created against main too.
There was a problem hiding this comment.
ATM the git project uses master as the trunk branch. My question was regarding main branch name in the list of the branches (where indeed master is present).
.github/workflows/release.yml
Outdated
| - name: Install python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.11' |
There was a problem hiding this comment.
@ignacio-penas Is there a way of getting rid of the version number here? Here is the doc https://github.com/marketplace/actions/setup-python
Maybe using .python-version file and reusing it in other places?
There was a problem hiding this comment.
@konstan I think this would be optimal since we already work with pyproject.toml files
There was a problem hiding this comment.
Version extraction working with pyproject.toml. It takes the latest from ">=3.9, <3.12". ATM is 3.11.8.
|
1 similar comment
|
|
| on: | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
ATM the git project uses master as the trunk branch. My question was regarding main branch name in the list of the branches (where indeed master is present).
| branches: | ||
| - "main" | ||
| - "master" | ||
|
|
There was a problem hiding this comment.
Same question here: ATM the git project uses master as the trunk branch. Did you add main just anticipating the future change of the trunk branch name from master to main?
|
@ignacio-penas looks good to me. The only thing that I found is that the badge in the readme will stop working after merge |
|
@0xbase12 Fixed to align with release.yml |
closes sixsq/tasklist#3005