Skip to content

Conversation

yanpenalva
Copy link
Contributor

Problem

The method getPermissions in the GateRegistrar class returns an array, which can lead to type compatibility issues when extending this class. The expected return type should be a Collection to match typical Laravel practices.

Solution

To resolve this issue, the getPermissions method has been modified to always return an instance of Collection. The method's logic has also been simplified by using Laravel's helpers and eliminating the need for an else statement.

Changes Made

  1. Ensure getPermissions returns a Collection: Adjusted the method to return a Collection.
  2. Simplified logic: Used a ternary operator to streamline the method.

Benefits

  • Ensures type consistency: Guarantees that the method returns the expected Collection type.
  • Simplified code: Makes the code cleaner and easier to read.

Please review this PR and tell me if everything is right so that the merge can be done.

@yanpenalva
Copy link
Contributor Author

@yajra How do I solve the error in PHP Linting so that the steps are approved? What is the default?

@yajra
Copy link
Owner

yajra commented Aug 5, 2024

@yanbrasiliano I will have to patch the Github actions. Some were disabled due to inactivity. PHP Linting is just Pint auto cs fixer.

@yanpenalva
Copy link
Contributor Author

@yanbrasiliano I will have to patch the Github actions. Some were disabled due to inactivity. PHP Linting is just Pint auto cs fixer.

@yajra Do I need to do anything to correct it before the assessment or is everything okay?

@yajra
Copy link
Owner

yajra commented Aug 5, 2024

The changes look good, I will have to test the changes on the actual app first. I will have this reviewed and merged within the week if all works well. Thanks!

@yajra yajra merged commit e547fff into yajra:master Aug 6, 2024
1 check failed
@yajra
Copy link
Owner

yajra commented Aug 6, 2024

Released on https://github.com/yajra/laravel-acl/releases/tag/v11.1.1 🚀 Thanks!

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.

2 participants