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. #77

Merged
merged 5 commits into from
Jun 25, 2024
Merged

Conversation

v16Studios
Copy link
Contributor

@v16Studios v16Studios commented Jun 18, 2024

Depends on: #73

PR Type

Enhancement, Dependencies, Configuration changes


Description

  • Reordered traits and removed HasEagerLimit trait in Beam, BeamClaim, and BeamScan models.
  • Updated GitHub Actions workflow to use GITHUB_TOKEN and corrected autoload command.
  • Updated PHP version requirement to ^8.2 and adjusted dependencies in composer.json.
  • Changed CACHE_DRIVER to CACHE_STORE in phpunit.xml and testbench.yaml.

Changes walkthrough 📝

Relevant files
Enhancement
Beam.php
Reordered traits and removed `HasEagerLimit` in Beam model.

src/Models/Laravel/Beam.php

  • Reordered trait usage for consistency.
  • Removed HasEagerLimit trait.
  • +2/-4     
    BeamClaim.php
    Removed `HasEagerLimit` trait in BeamClaim model.               

    src/Models/Laravel/BeamClaim.php

    • Removed HasEagerLimit trait.
    +0/-2     
    BeamScan.php
    Removed `HasEagerLimit` trait in BeamScan model.                 

    src/Models/Laravel/BeamScan.php

    • Removed HasEagerLimit trait.
    +0/-1     
    Configuration changes
    run_tests.yml
    Updated GitHub Actions workflow for token and autoload.   

    .github/workflows/run_tests.yml

  • Replaced COMPOSER_TOKEN with GITHUB_TOKEN.
  • Changed composer dumpautoload to composer dump-autoload.
  • +2/-2     
    phpunit.xml
    Updated cache environment variable in phpunit.xml.             

    phpunit.xml

    • Changed CACHE_DRIVER to CACHE_STORE.
    +1/-1     
    testbench.yaml
    Updated cache environment variable in testbench.yaml.       

    testbench.yaml

    • Changed CACHE_DRIVER to CACHE_STORE.
    +1/-1     
    Dependencies
    composer.json
    Updated dependencies and PHP version in composer.json.     

    composer.json

  • Updated PHP requirement to ^8.2.
  • Updated rebing/graphql-laravel to ^9.2.
  • Removed friendsofphp/php-cs-fixer.
  • Updated development dependencies.
  • +5/-7     

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

    @github-actions github-actions bot added enhancement New feature or request dependencies Pull requests that update a dependency file Configuration changes Review effort [1-5]: 3 labels Jun 18, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Dependency Update:
    Ensure that the updated dependencies in composer.json are compatible with each other and with the Laravel 11 framework.
    Trait Removal:
    Verify that the removal of HasEagerLimit trait does not affect the functionality of the models Beam, BeamClaim, and BeamScan.
    Configuration Changes:
    Check if the changes from CACHE_DRIVER to CACHE_STORE in phpunit.xml and testbench.yaml are correctly implemented across the application.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Verify the scope and permissions of the GITHUB_TOKEN used in GitHub Actions

    Ensure that replacing COMPOSER_TOKEN with GITHUB_TOKEN in the environment variables does
    not affect the authentication or permissions required for Composer operations. Verify that
    GITHUB_TOKEN has the necessary scopes.

    .github/workflows/run_tests.yml [46]

    -GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    +GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Verify scopes and permissions
     
    Suggestion importance[1-10]: 9

    Why: This is an important security suggestion to verify that the token used has the necessary permissions, which is crucial for the proper functioning of Composer operations in CI/CD pipelines.

    9
    Possible issue
    Review and test the updated rebing/graphql-laravel dependency for breaking changes

    Ensure that the updated dependency rebing/graphql-laravel from ^9.0.0-rc1 to ^9.2 does not
    introduce breaking changes or require additional modifications in your codebase. Review
    the changelog and test thoroughly.

    composer.json [28]

    -"rebing/graphql-laravel": "^9.2",
    +"rebing/graphql-laravel": "^9.2", # Review changelog and test updates
     
    Suggestion importance[1-10]: 8

    Why: The suggestion is important for maintaining stability and compatibility in the codebase. Reviewing changelogs and testing updates are best practices when updating dependencies.

    8
    Ensure the CACHE_STORE environment variable is correctly used in the application configuration

    Confirm that changing the environment variable from CACHE_DRIVER to CACHE_STORE aligns
    with the expected configuration keys used in your Laravel application, as this might
    affect cache functionality.

    phpunit.xml [27]

    -<env name="CACHE_STORE" value="redis"/>
    +<env name="CACHE_STORE" value="redis"/> <!-- Confirm key usage in config -->
     
    Suggestion importance[1-10]: 8

    Why: This is a valid suggestion to ensure that the environment variable change aligns with the application's configuration, which is crucial for maintaining cache functionality.

    8
    Reintroduce the HasEagerLimit trait to maintain eager loading functionality

    Consider reintroducing the HasEagerLimit trait if it was providing necessary functionality
    for handling eager loading limits in your models. If the removal was intentional and you
    have an alternative implementation, ensure it covers all previous use cases.

    src/Models/Laravel/Beam.php [24-25]

     use Traits\EagerLoadSelectFields;
     use Traits\HasBeamQr;
    +use Staudenmeir\EloquentEagerLimit\HasEagerLimit;
     
    Suggestion importance[1-10]: 7

    Why: The suggestion is valid as it highlights a potential issue with removing a trait that might be necessary for functionality. However, it is not critical if the removal was intentional and covered by alternative implementations.

    7

    enjinabner
    enjinabner previously approved these changes Jun 19, 2024
    enjinabner
    enjinabner previously approved these changes Jun 21, 2024
    @v16Studios v16Studios merged commit 8d572a4 into master Jun 25, 2024
    5 checks passed
    @v16Studios v16Studios deleted the update/pla-1678/laravel-11-update branch June 25, 2024 21:19
    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
    Labels
    Configuration changes dependencies Pull requests that update a dependency file enhancement New feature or request Review effort [1-5]: 3
    Development

    Successfully merging this pull request may close these issues.

    3 participants