Conversation
matancarmeli7
requested review from
ArbelNathan,
annaparienti,
barakda,
oriyarde,
roysahar-ibm,
tyichye19 and
zingero
…into task/CSI-2625_add_another_stage_before_the_merge
Signed-off-by: matancarmeli7 <matan.carmeli7@gmail.com>
Signed-off-by: matancarmeli7 <matan.carmeli7@gmail.com>
There was a problem hiding this comment.
I think we can avoid double assignment if we move this if to the first assignment
There was a problem hiding this comment.
what do you mean by "first assignment"?
There was a problem hiding this comment.
i.e.
triggering_branch = ${CI_ACTION_REF_NAME}
.
.
.
if [ "$triggering_branch" == "develop" ]; then
docker_image_branch_tag=latest
else
docker_image_branch_tag=`echo $driver_image_tags | awk '{print$2}'`
There was a problem hiding this comment.
but without backticks of course
There was a problem hiding this comment.
could you also put the triggering_branch = ${CI_ACTION_REF_NAME} line before the driver_image_tags=$(scripts/ci/get_image_tags_from_branch.sh ... line?
it's hard to decipher the name github chose for this env var
There was a problem hiding this comment.
also why is it sometimes docker_image... and not driver_image...?
Signed-off-by: matancarmeli7 <matan.carmeli7@gmail.com>
Signed-off-by: matancarmeli7 <matan.carmeli7@gmail.com>
Signed-off-by: matancarmeli7 <matan.carmeli7@gmail.com>
| - name: Set up Python | ||
| uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: '3.9.7' |
There was a problem hiding this comment.
could you remind me what is this for?
it's hard to tell which setup is required by which logic
There was a problem hiding this comment.
is it for docker/build-push-action?
| cache-to: type=local,dest=/tmp/.buildx-new-${{ matrix.image_type }} | ||
| # Temp fix | ||
| # CSI-3164 | ||
| # https://github.com/docker/build-push-action/issues/252 |
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v2 | ||
| - name: CSI-controller- static code analysis |
There was a problem hiding this comment.
| - name: CSI-controller- static code analysis | |
| - name: CSI-controller: static code analysis |
(same for unit testing, etc...)
could you align the names to the ones in the jenkinsfile, as much as possible?
unless you know of an easy way to maintain them in one place
There was a problem hiding this comment.
this would also mean:
k8s yamls validation should be:
K8s yamls validation (in both files)
3 participants
No description provided.