From b99d0432a7f0a803c492580085e85f81908e0dc1 Mon Sep 17 00:00:00 2001
From: Lukas Puehringer <lukas.puehringer@nyu.edu>
Date: Wed, 20 Apr 2022 14:56:27 +0200
Subject: [PATCH] build: minor updates in CI/CD workflow files

- polish code comments
- wrap long lines

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
---
 .github/workflows/cd.yml | 33 +++++++++++++--------------------
 .github/workflows/ci.yml |  2 +-
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml
index 1b38b1ae26..e36e6e3683 100644
--- a/.github/workflows/cd.yml
+++ b/.github/workflows/cd.yml
@@ -1,7 +1,7 @@
 name: CD
 concurrency: cd
 
-# Trigger workflow on completed CI (further checks below)
+# Trigger workflow on any completed CI (see further checks below)
 on:
   workflow_run:
     workflows: [CI]
@@ -11,18 +11,11 @@ jobs:
   build:
     name: Build
     runs-on: ubuntu-latest
-    # Skip unless CI was successful and ran on a ref starting with 'v' (release tag)
-    if: ${{ github.event.workflow_run.conclusion == 'success' && startsWith(github.event.workflow_run.head_branch, 'v') }}
-    # NOTE: This works because we currently only trigger CI on a push to the 'develop'
-    # branch or a 'v*'-tag, but it seems rather brittle.
-    # Unfortunately, there is not much more info we get from the CI workflow
-    # ('workflow_run') than the ref name. No ref, ref_type, etc., so we don't even know
-    # if a tag or a branch was pushed. :(
-    # See https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#workflow_run
-    # NOTE: (2) An alternative solution might be to restructure workflows, so that all
-    # test logic from 'ci.yml' is moved to a separate workflow file '_test.yml', that
-    # can be included in both CI (triggered on push to 'develop'-branch) and CD
-    # (triggered on push to 'v*'-tag) workflows.
+    # Skip unless CI was successful and ran on release tag, a ref starting with 'v'.
+    # NOTE: We assume CI does not trigger on branches that start with 'v' (see #1961)
+    if: >-
+      github.event.workflow_run.conclusion == 'success' &&
+      startsWith(github.event.workflow_run.head_branch, 'v')
     outputs:
       release_id: ${{ steps.gh-release.outputs.id }}
     steps:
@@ -52,13 +45,10 @@ jobs:
           files: dist/*
 
       - name: Store build artifacts
-        # NOTE: The release job could download the assets from the GitHub release page,
-        # published in the previous step. But using the GitHub upload/download actions
-        # seems more robust as there is no need to compute download URLs.
-        # NOTE: (2) action-gh-release returns download URLSs as output, which could be
-        # propagated to next job along with release_id (see above)
-        # https://github.com/softprops/action-gh-release#outputs
         uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535
+        # NOTE: The GitHub release page contains the release artifacts too, but using
+        # GitHub upload/download actions seems robuster: there is no need to compute
+        # download URLs and tampering with artifacts between jobs is more limited.
         with:
           name: build-artifacts
           path: dist
@@ -90,5 +80,8 @@ jobs:
               repo: context.repo.repo,
               release_id: '${{ needs.build.outputs.release_id }}',
               name: '${{ github.event.workflow_run.head_branch }}',
-              body: 'See [CHANGELOG.md](https://github.com/'+ context.repo.owner +'/'+ context.repo.repo +'/blob/${{ github.event.workflow_run.head_branch }}/docs/CHANGELOG.md) for details.'
+              body: 'See [CHANGELOG.md](https://github.com/' +
+                     context.repo.owner + '/' + context.repo.repo + '/blob/' +
+                     '${{ github.event.workflow_run.head_branch }}'+
+                     '/docs/CHANGELOG.md) for details.'
             })
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b7c9211f1c..b899d33ba4 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,11 +1,11 @@
 name: CI
 
 on:
+  # NOTE: CD relies on this configuration (see #1961)
   push:
     branches:
       - develop
     tags:
-      # TODO: Should we restrict to vX.Y.Z tags?
       - v*
 
   pull_request: