Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLA-1678] Laravel 11 update. #48

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

v16Studios
Copy link
Contributor

@v16Studios v16Studios commented Jun 18, 2024

PR Type

Enhancement, Configuration changes


Description

  • Updated GitHub Actions workflow to replace COMPOSER_TOKEN with GITHUB_TOKEN and corrected composer dumpautoload to composer dump-autoload.
  • Updated composer.json to require PHP ^8.2, updated rebing/graphql-laravel to ^9.2, added fix script for Laravel Pint, and updated development dependencies versions.
  • Updated phpunit.xml to change CACHE_DRIVER to CACHE_STORE in environment variables.
  • Updated testbench.yaml to change CACHE_DRIVER to CACHE_STORE in environment variables.

Changes walkthrough 📝

Relevant files
Configuration changes
run_tests.yml
Update GitHub Actions workflow for Laravel 11 compatibility.

.github/workflows/run_tests.yml

  • Replaced COMPOSER_TOKEN with GITHUB_TOKEN in environment variables.
  • Corrected composer dumpautoload to composer dump-autoload.
  • +2/-2     
    phpunit.xml
    Update PHPUnit configuration for Laravel 11.                         

    phpunit.xml

    • Changed CACHE_DRIVER to CACHE_STORE in environment variables.
    +1/-1     
    testbench.yaml
    Update Testbench configuration for Laravel 11.                     

    testbench.yaml

    • Changed CACHE_DRIVER to CACHE_STORE in environment variables.
    +1/-1     
    Enhancement
    composer.json
    Update dependencies and scripts for Laravel 11.                   

    composer.json

  • Updated PHP requirement to ^8.2.
  • Updated rebing/graphql-laravel to ^9.2.
  • Added fix script for Laravel Pint.
  • Updated development dependencies versions.
  • +7/-6     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review None

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Ensure consistent environment variable naming for cache settings

    Ensure that the CACHE_STORE environment variable change from CACHE_DRIVER is reflected
    consistently across all configurations to avoid runtime errors.

    phpunit.xml [27]

    -<env name="CACHE_STORE" value="redis"/>
    +<env name="CACHE_DRIVER" value="redis"/>
     
    Suggestion importance[1-10]: 8

    Why: This suggestion addresses a potential runtime error by ensuring consistent environment variable naming, which is crucial for avoiding misconfigurations and ensuring smooth operation.

    8
    Possible issue
    Restrict PHP version to avoid future compatibility issues

    Consider specifying a more restrictive version constraint for the php dependency to avoid
    potential compatibility issues with future PHP minor versions.

    composer.json [20]

    -"php": "^8.2",
    +"php": "~8.2.0",
     
    Suggestion importance[1-10]: 7

    Why: This suggestion improves compatibility and stability by restricting the PHP version to a specific minor version, reducing the risk of future compatibility issues.

    7
    Best practice
    Pin laravel/pint to a specific minor version for stability

    Update the laravel/pint dependency to a specific minor version instead of using the caret
    version range to ensure more predictable behavior and compatibility with the project.

    composer.json [73]

    -"laravel/pint": "^1.15",
    +"laravel/pint": "~1.15.0",
     
    Suggestion importance[1-10]: 6

    Why: Pinning the laravel/pint dependency to a specific minor version can enhance stability and predictability, but it is a minor improvement rather than a critical change.

    6
    Security
    Improve the security of the GitHub token usage

    Replace the direct usage of GITHUB_TOKEN with a more secure handling method, such as
    passing it through environment variables set in the workflow settings, to minimize the
    risk of accidental exposure.

    .github/workflows/run_tests.yml [46]

    -GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    +env:
    +  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     
    Suggestion importance[1-10]: 5

    Why: While the suggestion improves security by recommending a more secure handling method, the proposed change does not significantly alter the existing security posture since the token is already being sourced from GitHub secrets.

    5

    @leonardocustodio leonardocustodio merged commit d35f59a into master Jun 20, 2024
    5 checks passed
    @leonardocustodio leonardocustodio deleted the update/pla-1678/laravel-11-update branch June 20, 2024 09:54
    enjinabner pushed a commit that referenced this pull request Jun 26, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    3 participants