Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure: Use versions.yaml to read go version in e2e test #1509

Conversation

kartikjoshi21
Copy link
Contributor

Use versions.yaml to read go version in e2e test

Fixes: #1496

run: |
go_version="$(yq '.tools.golang' versions.yaml)"
[ -n "$go_version" ]
echo "GO_VERSION=${go_version}" >> "$GITHUB_ENV"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been tested? I think the env is just set for this job (read-properties), we need to use outputs if we want to carry variables from job to job, but why not just embed this in the build job?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this version is being used at multiple places it will be better to use output rather than reading file multiple times.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, that just adds more ceremony. I think the above version extraction can be condensed into a one liner and then it'll be just 2 lines per job (note the -e flag, that will make the job fail if unset)

- name: Extract go version number
   run: echo "GO_VERSION=$(yq -e '.tools.golang' versions.yaml)" >> "$GITHUB_ENV"

@kartikjoshi21 kartikjoshi21 force-pushed the kartikjoshi21/versions-e2e-test branch 2 times, most recently from 7681c41 to 789ffb8 Compare October 11, 2023 11:28
@kartikjoshi21 kartikjoshi21 force-pushed the kartikjoshi21/versions-e2e-test branch 2 times, most recently from c121870 to 13f4b0a Compare October 12, 2023 06:29
@kartikjoshi21 kartikjoshi21 requested a review from mkulke October 12, 2023 06:30
Copy link
Contributor

@katexochen katexochen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Should we also update the other places to the new one-liner?

@kartikjoshi21 kartikjoshi21 force-pushed the kartikjoshi21/versions-e2e-test branch from 13f4b0a to ef038c8 Compare October 16, 2023 06:48
@katexochen
Copy link
Contributor

The rebase introduced two new places in the file where we need to use the versions.yaml instead of the hard-coded string. Could you please update these, too?

Use versions.yaml to read go version in e2e test

Fixes: confidential-containers#1496
Signed-off-by: Kartik Joshi <[email protected]>
@kartikjoshi21 kartikjoshi21 force-pushed the kartikjoshi21/versions-e2e-test branch from ef038c8 to 3cfdec6 Compare October 25, 2023 10:17
@kartikjoshi21 kartikjoshi21 merged commit 4f2375b into confidential-containers:main Oct 25, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure-e2e-test: Use the go version from versions.yaml file
4 participants