From 2f0a26fd4e3f3ea24e9fb30f55a84ebbc194129c Mon Sep 17 00:00:00 2001 From: Cedric Date: Tue, 28 May 2024 13:36:00 +0200 Subject: [PATCH] Fix potential github action smells (#3770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hey! 🙂 I've made the following changes to your workflow: - Use cache parameter instead of cache option - Using the cache option from the setup/java is easier to configure and thus less prone to mistakes. Furthermore, it makes the workflow smaller and easier to read and understand. - Avoid jobs without timeouts - Jobs without a timeout can cause runners to be occupied when some process falls into an infinite loop or has multiple retries to perform a certain task, which will inevitably fail. Furthermore, it is also useful to be notified when a test-suite all of a sudden takes a lot longer than it used to, considering keeping tests fast is a good practice. (These changes are part of a research Study at TU Delft looking at GitHub Action Smells. [Find out more](https://ceddy4395.github.io/research/gha-smells.html)) --- .github/workflows/test_readme.yml | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test_readme.yml b/.github/workflows/test_readme.yml index ad7dc20f84..beacb3dc74 100644 --- a/.github/workflows/test_readme.yml +++ b/.github/workflows/test_readme.yml @@ -28,6 +28,7 @@ jobs: runs-on: ${{ matrix.os }} # skip commits made by the release plugin if: "!contains(github.event.head_commit.message, 'maven-release-plugin')" + timeout-minutes: 5 strategy: matrix: include: # Not ubuntu as already tested as a part of the docker job @@ -43,18 +44,13 @@ jobs: with: distribution: 'zulu' # zulu as it supports a wide version range java-version: '21' # Most recent LTS + cache: 'maven' - name: Cache NPM Packages uses: actions/cache@v4 with: path: ~/.npm # yamllint disable-line rule:line-length key: ${{ runner.os }}-npm-packages-${{ hashFiles('zipkin-lens/package-lock.json') }} - - name: Cache local Maven repository - uses: actions/cache@v4 - with: - path: ~/.m2/repository - key: ${{ runner.os }}-jdk-21-maven-${{ hashFiles('**/pom.xml') }} - restore-keys: ${{ runner.os }}-jdk-21-maven- - name: Execute Server Build # command from zipkin-server/README.md run: ./mvnw --also-make -pl zipkin-server clean package env: @@ -64,6 +60,7 @@ jobs: runs-on: ubuntu-22.04 # newest available distribution, aka jellyfish # skip commits made by the release plugin if: "!contains(github.event.head_commit.message, 'maven-release-plugin')" + timeout-minutes: 10 steps: - name: Checkout Repository uses: actions/checkout@v4 @@ -79,6 +76,7 @@ jobs: with: distribution: 'zulu' # zulu as it supports a wide version range java-version: '21' # Most recent LTS + cache: 'maven' # Don't attempt to cache Docker. Sensitive information can be stolen # via forks, and login session ends up in ~/.docker. This is ok because # we publish DOCKER_PARENT_IMAGE to ghcr.io, hence local to the runner. @@ -88,12 +86,6 @@ jobs: path: ~/.npm # yamllint disable-line rule:line-length key: ${{ runner.os }}-npm-packages-${{ hashFiles('zipkin-lens/package-lock.json') }} - - name: Cache local Maven repository - uses: actions/cache@v4 - with: - path: ~/.m2/repository - key: ${{ runner.os }}-jdk-21-maven-${{ hashFiles('**/pom.xml') }} - restore-keys: ${{ runner.os }}-jdk-21-maven- - name: Build zipkin-server # redundant, but needed for docker/README.md run: ./mvnw --also-make -pl zipkin-server clean package env: