-
Notifications
You must be signed in to change notification settings - Fork 74
Make our GH Actions source YML files more readable by factoring out strings #3328
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
base: master
Are you sure you want to change the base?
Changes from all commits
75c798f
875e0bd
6d2b945
6a362d8
5840e12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,10 +27,14 @@ on: | |
| required: false | ||
| type: string | ||
|
|
||
| env: | ||
| obs_project: ${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }} | ||
|
|
||
| jobs: | ||
| update_obs_package: | ||
| # do not run in forks which do not set the OBS_PROJECTS and OBS_USER variables, | ||
| # or the mapping for the current branch is missing | ||
| # (BTW, jobs.FOO.if cannot see env.*) | ||
| if: vars.OBS_PROJECTS && fromJson(vars.OBS_PROJECTS)[github.ref_name] && vars.OBS_USER | ||
|
|
||
| runs-on: ubuntu-latest | ||
|
|
@@ -66,27 +70,34 @@ jobs: | |
| OBS_USER: ${{ vars.OBS_USER }} | ||
| OBS_PASSWORD: ${{ secrets.OBS_PASSWORD }} | ||
|
|
||
| - name: Checkout ${{ inputs.package_name }} from ${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }} | ||
| run: osc co ${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }} ${{ inputs.package_name }} | ||
| - name: Checkout ${{ inputs.package_name }} from ${{ env.obs_project }} | ||
| run: osc co ${{ env.obs_project }} ${{ inputs.package_name }} | ||
|
|
||
| - name: Configure git | ||
| run: git config --global --add safe.directory "$GITHUB_WORKSPACE" | ||
|
|
||
| - name: Update service revision | ||
| # only when a tag has been pushed, or "release" branch updated | ||
| if: inputs.service_file != '' | ||
| run: |- | ||
| run: | | ||
| echo "Updating revision to \"${{ github.ref_name }}\"" | ||
| sed -i -e 's#<param name="revision">.*</param>#<param name="revision">${{ github.ref_name }}</param>#' ${{ inputs.service_file }} | ||
| BEG='<param name="revision">' | ||
| END='</param>' | ||
| sed -i -e "s#${BEG}.*${END}#${BEG}${{ github.ref_name }}${END}#" ${{ inputs.service_file }} | ||
|
|
||
| - name: Copy optional service file | ||
| # patch the URL in the file so it works also from forks, forks also by | ||
| # default do not inherit the tags so remove the version format option if | ||
| # no tag is present | ||
| if: inputs.service_file != '' | ||
| run: | | ||
| sed -e 's#<param name="url">.*</param>#<param name="url">https://github.com/${{ github.repository }}.git</param>#' ${{ inputs.service_file }} > ./${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }}/${{ inputs.package_name }}/_service | ||
| if [ -z "$(git tag -l)" ]; then sed -i -e 's#<param name="versionformat">.*</param>##' ./${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }}/${{ inputs.package_name }}/_service; fi | ||
| OUT_SERVICE=./${{ env.obs_project }}/${{ inputs.package_name }}/_service | ||
| BEG='<param name="url">' | ||
| END='</param>' | ||
| SED_EXPR="s#${BEG}.*${END}#${BEG}https://github.com/${{ github.repository }}.git${END}#" | ||
| sed -e "${SED_EXPR}" ${{ inputs.service_file }} > $OUT_SERVICE | ||
| BEG='<param name="versionformat">' | ||
| if [ -z "$(git tag -l)" ]; then sed -i -e "s#${BEG}.*${END}##" $OUT_SERVICE; fi | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if it is more readable now.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it is complex and dense, but IMHO better than the previous version
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, it makes reading a bit harder as you do not see what sed is in fact doing. I ask AI how to make it more readable and here is AI proposal which is for me more readable ( but still it includes sed expressions :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO this is a case where your using AI makes things (slightly) harder for your human counterparty, myself. Now I would have to transplant the result to the YAML file and retest it. If you trust the AI output, please make a patch or a commit for me :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, I do not trust AI, but paste it here to demonstrate what I do not like at your patch...that sed regexp like in some variable elsewhere, so you need more processing of it. On other hand AI solution ( even if it can have bugs ) is more easier to read for me as there is sed regexp with some long tricky replacements in own variable which is easier to read ( at least for me ). |
||
|
|
||
| - name: Run services | ||
| run: | | ||
|
|
@@ -95,13 +106,15 @@ jobs: | |
| # downloaded NPM package tarballs and they are accidentally added to the | ||
| # OBS package, so delete any TGZ files present | ||
| rm -vf *.tgz | ||
| working-directory: ./${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }}/${{ inputs.package_name }} | ||
| working-directory: ./${{ env.obs_project }}/${{ inputs.package_name }} | ||
|
|
||
| - name: Check status | ||
| run: osc addremove && osc diff && osc status | ||
| working-directory: ./${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }}/${{ inputs.package_name }} | ||
| working-directory: ./${{ env.obs_project }}/${{ inputs.package_name }} | ||
|
|
||
| - name: Commit ${{ inputs.package_name }} to ${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }} | ||
| run: |- | ||
| osc commit -m "Updated to $(sed -e '/^version:/!d' -e 's/version: *\(.*\)/\1/' agama.obsinfo) ($(sed -e '/^commit:/!d' -e 's/commit: *\(.*\)/\1/' agama.obsinfo))" | ||
| working-directory: ./${{ fromJson(vars.OBS_PROJECTS)[github.ref_name] }}/${{ inputs.package_name }} | ||
| - name: Commit ${{ inputs.package_name }} to ${{ env.obs_project }} | ||
| run: | | ||
| VERSION="$(sed -e '/^version:/!d' -e 's/version: *\(.*\)/\1/' agama.obsinfo)" | ||
| COMMIT="$(sed -e '/^commit:/!d' -e 's/commit: *\(.*\)/\1/' agama.obsinfo)" | ||
| osc commit -m "Updated to ${VERSION} (${COMMIT})" | ||
| working-directory: ./${{ env.obs_project }}/${{ inputs.package_name }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,14 +236,14 @@ workflows=(obs-staging-live.yml obs-staging-products.yml obs-staging-rust.yml ob | |
| if git ls-remote --exit-code --heads origin "$BRANCH" > /dev/null; then | ||
| for workflow in "${workflows[@]}"; do | ||
| echo "Starting GitHub Action $workflow..." | ||
| gh workflow run "$workflow" --ref "$BRANCH" | ||
| gh -R "$repo_slug" workflow run "$workflow" --ref "$BRANCH" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice..I fix that by using gh repo set-default, but your solution looks better. |
||
| done | ||
| else | ||
| echo "After creating the remote branch trigger the submission actions on the web" | ||
| echo "or run these commands:" | ||
| echo | ||
| for workflow in "${workflows[@]}"; do | ||
| echo " gh workflow run \"$workflow\" --ref \"$BRANCH\"" | ||
| echo " gh -R \"$repo_slug\" workflow run \"$workflow\" --ref \"$BRANCH\"" | ||
| done | ||
| fi | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be able using context. At least it is what doc says https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-variables#using-the-env-context-to-access-environment-variable-values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. That doc uses
envforjobs.<job_id>.steps.if, butenvis not available forjobs.<job_id>.if. Beautiful, right?https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#context-availability