#50 PHP-Scoper がテスト可能に #51
Conversation
|
🤖 OpenAI High-level SummaryThe changes include the addition of a GitHub Actions workflow to set up PHP Scoper for prefixing vendor files, modifications to the PHP-Scoper configuration, test setup modifications, and updates to the WordPress workflow. The changes aim to improve code organization, security, and maintainability by introducing automated processes for prefixing vendor files, loading plugins in tests, and checking PHP syntax. The PHP-Scoper configuration has been updated to exclude specific namespaces, classes, and functions. Files Summary
Chat with 🤖 OpenAI Bot (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Give access to tests_add_filter() function. | ||
| require_once $_tests_dir . '/includes/functions.php'; | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The comment on line 19 is unnecessary and can be removed.
| function _manually_load_plugin() { | ||
| require dirname( dirname( __FILE__ ) ) . '/ga-communicator.php'; | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The function _manually_load_plugin() is added to manually load the plugin being tested. This function should be documented with a PHPDoc block to describe its purpose, parameters, and return value.
/**
* Manually load the plugin being tested.
*/
function _manually_load_plugin() {
require dirname(dirname(__FILE__)) . '/ga-communicator.php';
}28:
The tests_add_filter() function is used to add a filter to load the plugin before running the tests. It would be helpful to provide a comment explaining the purpose of this filter.
// Load the plugin before running the tests.
tests_add_filter('muplugins_loaded', '_manually_load_plugin');Please ensure that the code snippets provided above are placed in the correct location within the file.
|
|
||
| // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#excluded-symbols | ||
| 'exclude-namespaces' => [ | ||
| '/^cli/', | ||
| '/^Composer/', | ||
| '/^DeepCopy/', | ||
| '/^Doctrine/', | ||
| '/^Kunoichi/', | ||
| '/^PharIo/', | ||
| '/^PHP_CodeSniffer/', | ||
| '/^PHPCompatibility/', | ||
| '/^PHPCSStandards/', | ||
| '/^PHPUnit/', | ||
| '/^Yoast/', | ||
| '/^SebastianBergmann/', | ||
| '/^TheSeer/', | ||
| '/^WP_/', | ||
| '/^Composer/', | ||
| '/^cli/' | ||
| ], |
There was a problem hiding this comment.
🤖 OpenAI
The exclude-namespaces array in the PHP-Scoper configuration has been modified. Several namespaces have been added to exclude from scoping. However, it seems that some of the namespaces were already present in the original configuration. Please verify if these additions are intentional and necessary.
| 'exclude-classes' => [ | ||
| '/^WP_/', | ||
| '/^Composer/' | ||
| '/^Composer/', | ||
| '/^PHPUnit/', | ||
| ], |
There was a problem hiding this comment.
🤖 OpenAI
The exclude-classes array in the PHP-Scoper configuration has been modified. Some classes have been added to exclude from scoping. Similar to the previous comment, please ensure that these additions are intentional and necessary.
| // | ||
| // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#exposed-symbols | ||
| 'expose-global-constants' => true, | ||
| 'expose-global-classes' => true, | ||
| 'expose-global-classes' => false, | ||
| 'expose-global-functions' => false, | ||
| 'expose-namespaces' => [ | ||
| ], |
There was a problem hiding this comment.
🤖 OpenAI
The new hunk modifies the values of 'expose-global-classes' and 'expose-global-functions' in the PHP-Scoper configuration file. The value of 'expose-global-classes' has been changed to false, and 'expose-global-functions' remains false. Without the complete context, it's difficult to determine if there are any issues with this change.
.github/workflows/wordpress.yml
Outdated
| php: [ '7.2', '7.4', '8.0' ] # PHP versions to check. | ||
| wp: [ 'latest', '5.9' ] # WordPress version to check. | ||
| tools: [ composer ] | ||
|
|
||
| scoper: [ true, false ] | ||
| services: | ||
| mysql: | ||
| image: mysql:8.0 |
There was a problem hiding this comment.
🤖 OpenAI
The services section in the workflow file has been modified. It now includes a mysql service with the image set to mysql:8.0.
.github/workflows/wordpress.yml
Outdated
| - name: Install dependencies | ||
| run: composer install --prefer-dist --no-progress --no-suggest | ||
|
|
||
| - uses: ./.github/workflows/php-scoper.yml | ||
| with: | ||
| scoper: ${{ matrix.scoper }} | ||
|
|
||
| - name: Make Binaries Executionable | ||
| if: matrix.scoper | ||
| run: chmod +x vendor-prefixed/vendor/bin/* | ||
|
|
||
| - name: Start MySQL | ||
| run: | | ||
| sudo systemctl start mysql | ||
| mysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';" | ||
|
|
||
|
|
||
| - name: Install WordPress | ||
| run: bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }} | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
Several steps have been added to the workflow. Here are the specific line ranges that need review:
| run: composer install --prefer-dist --no-progress --no-suggest | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The step installs dependencies using Composer. No issues found.
.github/workflows/wordpress.yml
Outdated
| - uses: ./.github/workflows/php-scoper.yml | ||
| with: | ||
| scoper: ${{ matrix.scoper }} |
There was a problem hiding this comment.
🤖 OpenAI
A new step is added to use the php-scoper.yml workflow. No issues found.
.github/workflows/wordpress.yml
Outdated
| - name: Make Binaries Executionable | ||
| if: matrix.scoper | ||
| run: chmod +x vendor-prefixed/vendor/bin/* | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
A new step named "Make Binaries Executionable" has been added. It runs the command chmod +x vendor-prefixed/vendor/bin/* if the scoper matrix variable is true. There are no issues with this change.
| run: | | ||
| sudo systemctl start mysql | ||
| mysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';" |
There was a problem hiding this comment.
🤖 OpenAI
A step is added to start MySQL and alter the root user's password. No issues found.
| - name: Install WordPress | ||
| run: bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }} |
There was a problem hiding this comment.
🤖 OpenAI
A step is added to install WordPress for testing purposes. No issues found.
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
A new step is added to build the package. No issues found.
.github/workflows/wordpress.yml
Outdated
| - uses: ./.github/workflows/php-scoper.yml | ||
| with: | ||
| scoper: true | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
A step is added to use the php-scoper.yml workflow with the scoper parameter set to true. No issues found.
121:
A step is added to clean unwanted files. No issues found.
Please review the changes and provide your feedback.
.github/workflows/php-scoper.yml
Outdated
| - uses: actions/checkout@master | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The actions/checkout@master step is missing a version specification. It's recommended to use a specific version of the actions/checkout action to ensure stability and avoid unexpected changes in behavior. Consider specifying a version, such as v2, for example:
- uses: actions/checkout@v2
.github/workflows/php-scoper.yml
Outdated
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer scoper-patch | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The commands for installing and running PHP-Scoper are executed directly on the runner without any checks or error handling. It's good practice to add error handling and validation for each command to ensure that they execute successfully. Additionally, it would be helpful to include some logging or output messages to indicate the progress and status of each command.
.github/workflows/php-scoper.yml
Outdated
| if: inputs.scoper | ||
| run: rm -rf vendor |
There was a problem hiding this comment.
🤖 OpenAI
The rm -rf vendor command is used to remove the vendor directory. However, this step is not safe and can potentially delete important files if the directory doesn't exist or if there are permission issues. It's recommended to add a check to ensure that the vendor directory exists before attempting to remove it. Additionally, consider using a safer method to remove the directory, such as using the rmdir command with appropriate options.
.github/workflows/php-scoper.yml
Outdated
| if: ! inputs.scoper | ||
| run: echo "Skipping Scoper" |
There was a problem hiding this comment.
🤖 OpenAI
The echo "Skipping Scoper" command is used to indicate that the PHP Scoper step is skipped. However, it would be more informative to include a log message indicating the reason for skipping, such as echo "PHP Scoper step skipped: inputs.scoper is false". This will provide better visibility and understanding of the workflow execution.
.github/workflows/php-scoper.yml
Outdated
| - uses: actions/checkout@master | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The actions/checkout@master step is missing the version specification. It's recommended to specify a specific version of the action to ensure stability and avoid unexpected changes in behavior. Consider updating the step to use a specific version, such as actions/checkout@v2.
| name: Setup PHP Scoper | ||
| description: Setup PHP Scoper | ||
| inputs: |
There was a problem hiding this comment.
🤖 OpenAI
The name and description fields in the GitHub Actions workflow file should be more descriptive. Consider updating them to provide clearer information about the purpose of the workflow.
| runs: | ||
| using: "composite" |
There was a problem hiding this comment.
🤖 OpenAI
The runs section of the workflow is using the composite strategy, which allows running multiple steps. However, there is only one step defined in this workflow. If there are no plans to add additional steps in the future, you can remove the using: "composite" line.
| - name: Add Prefix | ||
| if: inputs.scoper | ||
| shell: bash | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer scoper-patch | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The current implementation assumes that the php-scoper binary is available globally. However, this may not always be the case. It would be better to use the composer binary to execute the php-scoper command. Replace lines 15-18 with the following code:
| - name: Add Prefix | |
| if: inputs.scoper | |
| shell: bash | |
| run: | | |
| composer global require humbug/php-scoper | |
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | |
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | |
| composer scoper-patch | |
| - name: Add Prefix | |
| if: inputs.scoper | |
| run: | | |
| composer require humbug/php-scoper --dev | |
| composer exec -- php-scoper add-prefix --output-dir=vendor-prefixed --force | |
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | |
| composer scoper-patch |
| - name: Cleanup Vendor | ||
| if: inputs.scoper | ||
| run: rm -rf vendor | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The cleanup of the vendor directory should only be performed if the PHP Scoper step is enabled. Move the if condition to line 21 and remove the shell: bash attribute from line 23. The updated code should look like this:
| - name: Cleanup Vendor | |
| if: inputs.scoper | |
| run: rm -rf vendor | |
| shell: bash | |
| - name: Cleanup Vendor | |
| if: inputs.scoper | |
| run: rm -rf vendor |
.github/workflows/wordpress.yml
Outdated
| - name: Install dependencies | ||
| run: composer install --prefer-dist --no-progress --no-suggest | ||
|
|
||
| - uses: ./.github/actions/php-scoper/action.yml | ||
| name: Do PHP Scoper if possible | ||
| with: | ||
| scoper: ${{ matrix.scoper }} | ||
|
|
||
| - name: Make Binaries Executionable | ||
| if: matrix.scoper | ||
| run: chmod +x vendor-prefixed/vendor/bin/* | ||
|
|
||
| - name: Start MySQL | ||
| run: | | ||
| sudo systemctl start mysql | ||
| mysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';" | ||
|
|
||
|
|
||
| - name: Install WordPress | ||
| run: bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }} | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
Several steps have been added to the workflow file. Here are the specific line ranges and their corresponding issues:
.github/workflows/wordpress.yml
Outdated
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build Package. | ||
| - name: Build Package | ||
| run: bash bin/build.sh | ||
|
|
||
| - name: Do PHP scoper | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer help-scoper | ||
| - uses: ./.github/actions/php-scoper/action.yml | ||
| name: Do PHP Scoper | ||
| with: | ||
| scoper: true | ||
|
|
||
| - name: Clean Unwanted files | ||
| run: bash bin/clean.sh |
There was a problem hiding this comment.
🤖 OpenAI
More steps have been added to the workflow. Here are the specific issues:
- Line 115: The
bash bin/build.shcommand is used to build a package. Make sure this script exists and is correctly configured. - Line 118: The
./.github/actions/php-scoper/action.ymlfile is being used as an action. Make sure this file exists and contains the necessary configuration. - Line 120: The
scoperinput is being passed to the action, but it's not clear where this value is coming from. Ensure that the correct value is being passed here. - Line 123: The
bash bin/clean.shcommand is used to clean unwanted files. Make sure this script exists and is correctly configured.
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Build Package. | |
| - name: Build Package | |
| run: bash bin/build.sh | |
| - name: Do PHP scoper | |
| run: | | |
| composer global require humbug/php-scoper | |
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | |
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | |
| composer help-scoper | |
| - uses: ./.github/actions/php-scoper/action.yml | |
| name: Do PHP Scoper | |
| with: | |
| scoper: true | |
| - name: Clean Unwanted files | |
| run: bash bin/clean.sh | |
| <!-- This is an auto-generated comment by OpenAI --> |
.github/workflows/wordpress.yml
Outdated
| MYSQL_AUTHENTICATION_PLUGIN: mysql_native_password | ||
| name: UnitTest on WordPress ${{ matrix.wp }} in PHP ${{ matrix.php }} | ||
| steps: |
There was a problem hiding this comment.
🤖 OpenAI
A new step named "Setup PHP with composer v2" has been added to the workflow. It uses the shivammathur/setup-php@v2 action.
.github/workflows/wordpress.yml
Outdated
| - name: Install dependencies | ||
| run: composer install --prefer-dist --no-progress --no-suggest | ||
|
|
||
| - uses: ./.github/actions/php-scoper | ||
| name: Do PHP Scoper if possible | ||
| with: | ||
| scoper: ${{ matrix.scoper }} | ||
|
|
||
| - name: Make Binaries Executionable | ||
| if: matrix.scoper | ||
| run: chmod +x vendor-prefixed/vendor/bin/* | ||
|
|
||
| - name: Start MySQL | ||
| run: | | ||
| sudo systemctl start mysql | ||
| mysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';" | ||
|
|
||
|
|
||
| - name: Install WordPress | ||
| run: bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }} | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
Several steps have been added to the workflow. Here are the key changes:
- Line 48: The step "Install dependencies" now runs the command
composer install --prefer-dist --no-progress --no-suggest. - Lines 50-53: A new step named "Do PHP Scoper if possible" has been added. It uses a custom action located at
./.github/actions/php-scoperand takes a parameterscoperfrom the matrix. - Lines 55-57: A new step named "Make Binaries Executionable" has been added. It runs the command
chmod +x vendor-prefixed/vendor/bin/*only if thescoperparameter is true. - Lines 59-63: The step "Start MySQL" has been modified to include two commands:
sudo systemctl start mysqlandmysql -h 127.0.0.1 --port 3306 -u root --password=root -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';". - Line 65: The step "Install WordPress" now runs the command
bash bin/install-wp-tests.sh wordpress root root 127.0.0.1:3306 ${{ matrix.wp }}.
.github/workflows/wordpress.yml
Outdated
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build Package. | ||
| - name: Build Package | ||
| run: bash bin/build.sh | ||
|
|
||
| - name: Do PHP scoper | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer help-scoper | ||
| - uses: ./.github/actions/php-scoper | ||
| name: Do PHP Scoper | ||
| with: | ||
| scoper: true | ||
|
|
||
| - name: Clean Unwanted files | ||
| run: bash bin/clean.sh |
There was a problem hiding this comment.
🤖 OpenAI
Several steps have been added to the "Build on Tags" job. Here are the key changes:
- Line 117: A new step named "Do PHP Scoper" has been added. It uses a custom action located at
./.github/actions/php-scoperand sets thescoperparameter to true. - Line 122: The step "Clean Unwanted files" now runs the command
bash bin/clean.sh.
Overall, the changes in the workflow file seem fine. However, it's important to review the contents of the custom actions (./.github/actions/php-scoper) to ensure they align with the desired behavior.
| name: Setup PHP Scoper | ||
| description: Setup PHP Scoper | ||
| inputs: | ||
| scoper: | ||
| description: Whether to do PHP Scoper | ||
| required: true |
There was a problem hiding this comment.
🤖 OpenAI
The scoper input is marked as required, but it seems that the condition for executing the PHP Scoper steps is based on the value 'yes' instead of 'true'. This could lead to confusion and potential issues. It would be better to align the condition with the input description and make it consistent. Consider changing line 6 to:
required: true
default: 'no'And changing lines 12 and 21 to:
if: ${{ inputs.scoper == 'yes' }}This ensures that the condition matches the input value and avoids any ambiguity.
| - name: Skip PHP Scoper | ||
| if: inputs.scoper != true | ||
| run: echo "Skipping Scoper" | ||
| shell: bash |
There was a problem hiding this comment.
🤖 OpenAI
The condition on line 26 should be modified to check if the scoper input is set to false. Update the condition on line 26 to inputs.scoper == 'false'. Additionally, remove the shell: bash attribute from line 28. The updated code should look like this:
| - name: Skip PHP Scoper | |
| if: inputs.scoper != true | |
| run: echo "Skipping Scoper" | |
| shell: bash | |
| - name: Skip PHP Scoper | |
| if: inputs.scoper == 'false' | |
| run: echo "Skipping Scoper" |
Overall, the changes made to .github/actions/php-scoper/action.yml are as follows:
1: name: Setup PHP Scoper
2: description: Setup PHP Scoper
3: inputs:
4: scoper:
5: description: Whether to do PHP Scoper
6: required: false
7:
8: runs:
9: using: "composite"
10: steps:
11: - name: Add Prefix
12: if: inputs.scoper
13: run: |
14: composer require humbug/php-scoper --dev
15: composer exec -- php-scoper add-prefix --output-dir=vendor-prefixed --force
16: composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative
17: composer scoper-patch
18:
19: - name: Cleanup Vendor
20: if: inputs.scoper
21: run: rm -rf vendor
22:
23: - name: Skip PHP Scoper
24: if: inputs.scoper == 'false'
25: run: echo "Skipping Scoper"Note: Please ensure that the composer binary is available in the environment where this workflow will be executed.
| ] ), | ||
| ], | ||
|
|
||
| // List of excluded files, i.e. files for which the content will be left untouched. | ||
| // Paths are relative to the configuration file unless if they are already absolute | ||
| // | ||
| // For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#patchers | ||
| 'exclude-files' => [ | ||
| 'src/a-whitelisted-file.php', | ||
| ], | ||
|
|
||
| // When scoping PHP files, there will be scenarios where some of the code being scoped indirectly references the | ||
| // original namespace. These will include, for example, strings or string manipulations. PHP-Scoper has limited | ||
| // support for prefixing such strings. To circumvent that, you can define patchers to manipulate the file to your |
There was a problem hiding this comment.
🤖 OpenAI
The comment in the new hunk provides a good explanation of the purpose of the patchers. However, it seems that the old hunk is missing some lines of code that were removed. Without the complete context, it's difficult to determine if there are any issues with this change.
|
|
||
| // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#excluded-symbols | ||
| 'exclude-namespaces' => [ | ||
| '/^cli/', | ||
| '/^Composer/', | ||
| '/^DeepCopy/', | ||
| '/^Doctrine/', | ||
| '/^Kunoichi/', | ||
| '/^PharIo/', | ||
| '/^PHP_CodeSniffer/', | ||
| '/^PHPCompatibility/', | ||
| '/^PHPCSStandards/', | ||
| '/^PHPUnit/', | ||
| '/^Yoast/', | ||
| '/^SebastianBergmann/', | ||
| '/^TheSeer/', | ||
| '/^WP_/', | ||
| '/^Composer/', | ||
| '/^cli/' | ||
| ], | ||
| 'exclude-classes' => [ | ||
| '/^WP_/', | ||
| '/^Composer/' | ||
| '/^Composer/', | ||
| '/^PHPUnit/', | ||
| ], | ||
| 'exclude-functions' => [ | ||
| // 'mb_str_split', |
There was a problem hiding this comment.
🤖 OpenAI
The new hunk updates the excluded namespaces, classes, and functions in the PHP-Scoper configuration file. It appears that the changes are intended to exclude additional namespaces, classes, and functions from being scoped. Without the complete context, it's difficult to determine if there are any issues with this change.
| - name: Add Prefix | ||
| if: 'yes' == inputs.scoper | ||
| shell: bash | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer scoper-patch | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The current implementation assumes that the PHP-Scoper tool is already installed globally. It would be better to include a step to install the tool if it's not already available. Additionally, the --output-dir option in the php-scoper add-prefix command should be set to vendor/vendor-prefixed instead of just vendor-prefixed. This ensures that the prefixed vendor files are placed in the correct directory.
11: - name: Install PHP Scoper
12: if: 'yes' == inputs.scoper
13: run: composer global require humbug/php-scoper
14:
15: - name: Add Prefix
16: if: 'yes' == inputs.scoper
17: shell: bash
18: run: |
19: ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor/vendor-prefixed --force
20: composer dump-autoload --working-dir vendor/vendor-prefixed --classmap-authoritative
21: composer scoper-patch
| - name: Cleanup Vendor | ||
| if: 'yes' == inputs.scoper | ||
| run: rm -rf vendor | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The cleanup step (Cleanup Vendor) should only be executed if PHP Scoper is enabled (inputs.scoper == 'yes'). Otherwise, it could unintentionally delete the entire vendor directory, leading to issues during subsequent steps. Consider modifying lines 20-23 as follows:
- name: Cleanup Vendor
if: ${{ inputs.scoper == 'yes' }}
run: rm -rf vendor
shell: bashThis change ensures that the cleanup step is conditional and only executed when necessary.
| - name: Skip PHP Scoper | ||
| if: ${{ inputs.scoper }} != 'no' | ||
| run: echo "Skipping Scoper" | ||
| shell: bash |
There was a problem hiding this comment.
🤖 OpenAI
The Skip PHP Scoper step is unnecessary and can be removed. Since it comes after the previous steps, it will always be skipped if PHP Scoper is not enabled.
| name: Setup PHP Scoper | ||
| description: Setup PHP Scoper | ||
| inputs: | ||
| scoper: | ||
| description: Whether to do PHP Scoper | ||
| required: true | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The scoper input parameter should have a default value of false instead of being required. This allows the workflow to run without explicitly specifying the scoper input. Additionally, the description for the scoper input should be updated to indicate that it controls whether PHP Scoper is enabled or not.
3: inputs:
4: scoper:
5: description: Whether to enable PHP Scoper
6: required: false
7: default: false
| if: 'yes' == inputs.scoper | ||
| run: rm -rf vendor | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The Cleanup Vendor step should be moved outside the if condition so that it always runs, regardless of whether PHP Scoper is enabled or not. This ensures that the vendor directory is cleaned up properly.
20: - name: Cleanup Vendor
21: run: rm -rf vendor
22: shell: bash
23:
24: - name: Skip PHP Scoper
| if: inputs.scoper != 'yes' | ||
| run: echo "Skipping Scoper" | ||
| shell: bash |
There was a problem hiding this comment.
🤖 OpenAI
The Skip PHP Scoper step can be simplified by removing the if condition and the shell attribute. The command can be directly executed without the need for a separate shell.
25: - name: Skip PHP Scoper
26: run: echo "Skipping Scoper"Overall, here are the suggested changes:
1: name: Setup PHP Scoper
2: description: Setup PHP Scoper
3: inputs:
4: scoper:
5: description: Whether to enable PHP Scoper
6: required: false
7: default: false
8:
9: runs:
10: using: "composite"
11: steps:
12: - name: Install PHP Scoper
13: if: 'yes' == inputs.scoper
14: run: composer global require humbug/php-scoper
15:
16: - name: Add Prefix
17: if: 'yes' == inputs.scoper
18: shell: bash
19: run: |
20: ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor/vendor-prefixed --force
21: composer dump-autoload --working-dir vendor/vendor-prefixed --classmap-authoritative
22: composer scoper-patch
23:
24: - name: Cleanup Vendor
25: run: rm -rf vendor
26:
27: - name: Skip PHP Scoper
28: run: echo "Skipping Scoper"
| name: Setup PHP Scoper | ||
| description: Setup PHP Scoper | ||
| inputs: | ||
| scoper: | ||
| description: Whether to do PHP Scoper | ||
| required: true | ||
|
|
||
| runs: | ||
| using: "composite" |
There was a problem hiding this comment.
🤖 OpenAI
The new hunk introduces a GitHub Actions workflow file for setting up PHP Scoper. It includes inputs for the scoper parameter and defines multiple steps to execute based on the value of inputs.scoper. The workflow runs as a composite action.
| - name: Add Prefix | ||
| if: inputs.scoper == 'yes' | ||
| shell: bash | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer scoper-patch | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
In the step named "Add Prefix," the script installs the humbug/php-scoper package globally using Composer. It then executes the php-scoper add-prefix command to prefix the vendor files and outputs them to the vendor-prefixed directory. After that, it runs composer dump-autoload with the --working-dir option set to vendor-prefixed and --classmap-authoritative to regenerate the autoload files. Finally, it runs composer scoper-patch.
| - name: Cleanup Vendor | ||
| if: inputs.scoper == 'yes' | ||
| run: rm -rf vendor | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The step named "Cleanup Vendor" removes the existing vendor directory if inputs.scoper is set to 'yes'.
| - name: Skip PHP Scoper | ||
| if: inputs.scoper != 'yes' | ||
| run: echo "Skipping Scoper" | ||
| shell: bash |
There was a problem hiding this comment.
🤖 OpenAI
The step named "Skip PHP Scoper" simply echoes "Skipping Scoper" if inputs.scoper is not set to 'yes'.
Overall, the changes look good. However, there are a few suggestions:
-
In the step named "Add Prefix," it would be helpful to include a comment explaining what the purpose of each command is. This will make it easier for future maintainers to understand the workflow.
-
Consider adding error handling to the commands in the "Add Prefix" step. If any of the commands fail, it would be useful to have appropriate error messages or exit codes to indicate the failure.
-
In the step named "Cleanup Vendor," it would be better to use the
rmcommand before theshellattribute instead of after it. -
Consider adding error handling to the "Cleanup Vendor" step as well. If the removal of the
vendordirectory fails, it would be helpful to have appropriate error messages or exit codes. -
In the step named "Skip PHP Scoper," consider adding a comment explaining why PHP Scoper is being skipped when
inputs.scoperis not set to 'yes'.
Here's an example of how the changes could be made:
| - name: Add Prefix | ||
| if: inputs.scoper == 'yes' | ||
| shell: bash | ||
| run: | | ||
| composer global require humbug/php-scoper | ||
| ~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force | ||
| composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative | ||
| composer scoper-patch | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
Add comments explaining the purpose of each command and add error handling.
- name: Add Prefix
if: inputs.scoper == 'yes'
shell: bash
run: |
# Install humbug/php-scoper globally
composer global require humbug/php-scoper
# Prefix vendor files
~/.composer/vendor/bin/php-scoper add-prefix --output-dir=vendor-prefixed --force || { echo "Failed to prefix vendor files"; exit 1; }
# Regenerate autoload files
composer dump-autoload --working-dir vendor-prefixed --classmap-authoritative || { echo "Failed to regenerate autoload files"; exit 1; }
# Apply scoper patch
composer scoper-patch || { echo "Failed to apply scoper patch"; exit 1; }
| - name: Cleanup Vendor | ||
| if: inputs.scoper == 'yes' | ||
| run: rm -rf vendor | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
Move the rm command before the shell attribute.
- name: Cleanup Vendor
if: inputs.scoper == 'yes'
run: |
rm -rf vendor
shell: bash
| - name: Skip PHP Scoper | ||
| if: inputs.scoper != 'yes' | ||
| run: echo "Skipping Scoper" | ||
| shell: bash |
There was a problem hiding this comment.
🤖 OpenAI
Add a comment explaining why PHP Scoper is being skipped.
- name: Skip PHP Scoper
if: inputs.scoper != 'yes'
run: echo "Skipping Scoper" # PHP Scoper is not enabled
shell: bashThese suggestions will improve the clarity and robustness of the workflow.
Summary by OpenAI
Release Notes:
actions/checkoutaction to version 3 in the WordPress workflow.