Skip to content

Conversation

@crazywhalecc
Copy link
Owner

What does this PR do?

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php, run composer cs-fix at local machine.
  • If it's an extension or dependency update, make sure adding related extensions in src/global/test-extensions.php.
  • If you changed the behavior of static-php-cli, update docs in ./docs/.
  • If you updated config/xxx.json content, run bin/spc dev:sort-config xxx.

{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
@crazywhalecc crazywhalecc requested a review from Copilot June 13, 2025 18:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces default handling for shared extensions, updates documentation to reflect the new option, and adds a GitHub Actions workflow to parse commit message tags and run craft/bash tests automatically.

  • Provide an empty-array default for shared-extensions in CraftCommand
  • Document the shared-extensions key under build-options
  • Add .github/workflows/commit-tests.yml to conditionally run tests based on {craft} and {bash} tags

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/SPC/command/CraftCommand.php Default missing shared-extensions to an empty array
docs/deps-craft-yml.md Add shared-extensions: [] sample under build-options
.github/workflows/commit-tests.yml New workflow to parse commit tags and run craft/bash tests
Comments suppressed due to low confidence (6)

src/SPC/command/CraftCommand.php:52

  • Add or update unit/integration tests to cover the scenario when shared-extensions is not provided, ensuring that the default empty array case behaves as expected.
$shared_extensions = implode(',', $craft['shared-extensions'] ?? []);

docs/deps-craft-yml.md:45

  • Indent this comment line and the following key so they correctly appear under the build-options block with the same two-space indentation as the other YAML entries.
# Build options for shared extensions (same as `build-shared` command options, all options are optional)

.github/workflows/commit-tests.yml:35

  • This exit 0 halts the entire parsing script when {craft} tags are missing, preventing downstream parsing of Bash tags or prefix/OS settings. Consider removing or replacing it to allow the script to continue processing.
            exit 0

.github/workflows/commit-tests.yml:36

  • Explicitly set skip_craft=no when craft tags are found to make the workflow logic clearer and avoid relying on an empty default.
          else

.github/workflows/commit-tests.yml:51

  • In the Bash parsing block, explicitly set skip_bash=no when {bash} tags are found to make the condition more transparent.
          else

.github/workflows/commit-tests.yml:1

  • [nitpick] The workflow name is Single Test but it runs both craft and bash jobs. Consider renaming it to something like Commit Tests to better reflect its purpose.
name: Single Test

{craft}
extensions: bcmath
shared-extensions: xdebug,swoole
sapi: cli
{/craft}
[spc_prefix:bin/spc-gnu-docker]
@henderkes
Copy link
Collaborator

I'm on a short weekend trip, I can leave a proper review tomorrow evening

@crazywhalecc
Copy link
Owner Author

I'm on a short weekend trip, I can leave a proper review tomorrow evening

Ah no problem. I'll leave tomorrow, too. Enjoy your weekend!

@crazywhalecc crazywhalecc requested a review from henderkes June 18, 2025 03:54
@crazywhalecc
Copy link
Owner Author

Looks good to me now. I also updated the PR template.

@crazywhalecc crazywhalecc merged commit 24e19de into main Jun 18, 2025
140 checks passed
@crazywhalecc crazywhalecc deleted the ci/commit-tests branch June 18, 2025 06:16
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.

3 participants