Skip to content

feat(setup-install): create new action#349

Open
damienwebdev wants to merge 1 commit intomage-os:mainfrom
damienwebdev:setup-install
Open

feat(setup-install): create new action#349
damienwebdev wants to merge 1 commit intomage-os:mainfrom
damienwebdev:setup-install

Conversation

@damienwebdev
Copy link
Copy Markdown
Member

@damienwebdev damienwebdev commented May 5, 2026

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Currently, users have to awkward run setup:install themselves based upon matrix configuration. This sucks.

Fixes: #350

What is the new behavior?

Users can user setup-install with the services matrix configuration and Magento will install based upon that service config.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@damienwebdev damienwebdev requested a review from a team as a code owner May 5, 2026 20:19
@damienwebdev
Copy link
Copy Markdown
Member Author

This is blocked by #351

@rhoerr
Copy link
Copy Markdown
Contributor

rhoerr commented May 6, 2026

351 merged; any changes or rebase needed here?

@damienwebdev
Copy link
Copy Markdown
Member Author

damienwebdev commented May 6, 2026

This wound up blocked by #353 -- this will need a rebase once merged.

@rhoerr
Copy link
Copy Markdown
Contributor

rhoerr commented May 7, 2026

Code review

Found 2 issues:

  1. README usage example references the wrong repository name (mage-os/github-actions-magento2). The repo is mage-os/github-actions; every other action's README in this repo uses mage-os/github-actions/<action>@main. Users copy-pasting this example will hit a non-existent path.

steps:
- uses: actions/checkout@v6
- uses: mage-os/github-actions-magento2/supported-version@main
id: supported-version
with:
include_services: "true"
install:
needs: compute_matrix
runs-on: ${{ matrix.os }}
services: ${{ matrix.services }}
strategy:
fail-fast: false
matrix: ${{ fromJSON(needs.compute_matrix.outputs.matrix) }}
steps:
- uses: actions/checkout@v6
- uses: mage-os/github-actions-magento2/setup-magento@main
id: setup-magento
with:
php-version: ${{ matrix.php }}
tools: composer:v${{ matrix.composer }}
env:
COMPOSER_AUTH: ${{ secrets.COMPOSER_AUTH }}
- run: composer install
working-directory: ${{ steps.setup-magento.outputs.path }}
env:
COMPOSER_AUTH: ${{ secrets.COMPOSER_AUTH }}
- uses: mage-os/github-actions-magento2/setup-install@main
with:
services: ${{ toJSON(matrix.services) }}

  1. The Services type in setup-install is redefined as a closed interface with named optional keys, while the canonical definition in supported-version/src/matrix/matrix-type.ts is an open index signature { [serviceName: string]: ServiceConfig }. Two divergent definitions of the same concept will drift. Consider exporting/sharing the type from supported-version instead, so adding a future service (e.g. varnish) is type-checked across actions.

export interface Services {
mysql?: ServiceConfig;
opensearch?: ServiceConfig;
elasticsearch?: ServiceConfig;
rabbitmq?: ServiceConfig;
redis?: ServiceConfig;
valkey?: ServiceConfig;
}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@rhoerr
Copy link
Copy Markdown
Contributor

rhoerr commented May 7, 2026

I did a visual inspection then ran the above. The services feedback might not be meaningful. Possibly best to have them fully isolated in this way. The readme reference issue looks valid.

I saw you have the same or similar at https://github.com/graycoreio/github-actions-magento2/tree/main/setup-install . All good there?

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.

[MAINT] Github Actions Reunification - setup-install

2 participants