Skip to content

GH-641: Fix Known CI Issues#658

Open
Infinite-Null wants to merge 23 commits intotheme-elementary-v2from
fix/ci-known-issues
Open

GH-641: Fix Known CI Issues#658
Infinite-Null wants to merge 23 commits intotheme-elementary-v2from
fix/ci-known-issues

Conversation

@Infinite-Null
Copy link
Copy Markdown
Member

@Infinite-Null Infinite-Null commented Apr 1, 2026

Description

Resolves multiple issues in the CI workflow that were causing incorrect behavior and deprecation warnings.
Fixes: #641

Technical Details

  • Moved the "Notice" logic into the pre-run check to centralize file-detection logic and reduce redundancy.

  • Removed changed-gha-workflow-count and changed-file-count from the outputs block. Stripped the || needs.pre-run.outputs.changed-gha-workflow-count > 0 conditions from all job.

  • Maintained GHA_WORKFLOW_COUNT and MODIFIED_FILES_DATA inside the determine-file-counts step for logging and visibility purposes, without exporting them to downstream jobs.

  • Replaced all deprecated ::set-output usages with the recommended $GITHUB_OUTPUT approach to ensure compatibility with current GitHub Actions standards.

  • Eliminated hard-coded "CK theme" references in log messages. These have been replaced with dynamic values using ${{ github.event.repository.name }}, improving reusability across repositories.

  • Corrected the logic of the Notice step in the unit-test-php job. Previously, it executed when PHP files were modified; it now correctly runs only when no PHP changes are detected.

  • Introduced dependency caching for node_modules using actions/cache@v5.0.3

    The cache key is composed of:

    • OS
    • package-lock.json hash
    • Node version

Checklist

  • Deprecated ::set-output syntax → replaced with $GITHUB_OUTPUT
  • Hard-coded client name ("CK theme") → replaced with dynamic repository name
  • Incorrect conditional logic in Notice step → fixed execution condition
  • Missing npm dependency caching → added actions/cache for node_modules

Screenshots

image Screenshot 2026-04-01 at 7 14 17 PM

@aryanjasala aryanjasala changed the base branch from main to theme-elementary-v2 April 2, 2026 08:31
key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}-${{ steps.node.outputs.version }}

- name: Install Node dependencies
if: needs.pre-run.outputs.changed-php-count > 0 && steps.node_modules.outputs.cache-hit != 'true'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are node dependencies dependent on needs.pre-run.outputs.changed-php-count?

Copy link
Copy Markdown
Member Author

@Infinite-Null Infinite-Null Apr 3, 2026

Choose a reason for hiding this comment

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

The condition needs.pre-run.outputs.changed-php-count > 0 is added to avoid unnecessary Node installation when PHP tests are not required.

In this workflow, the unit-test-php job cannot be skipped entirely due to required status checks, so it still runs even when no PHP files are changed. Because of that, without this condition:

The cache step would still execute
And on a cache miss, npm ci would run
This would install Node dependencies even though PHP tests are not needed

So the condition ensures that Node-related setup (cache + install) only runs when PHP files have actually changed, preventing unnecessary installs caused by cache misses in irrelevant runs.

As per the latest change, this has been removed.

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.

Fix Known CI Issues

2 participants