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

Replace php-cs-fixer with Laravel Pint #2780

Merged
merged 3 commits into from
Dec 26, 2024
Merged

Conversation

bobbrodie
Copy link
Contributor

@bobbrodie bobbrodie commented Dec 26, 2024

Overview

I've started working with this package, and wanted to take a deeper look at how it's built, so I was reviewing the source. One small thing I came upon was that there was a Composer script that would call php-cs-fixer fix --allow-risky=yes. There were two issues:

  1. The package did not include the friendsofphp/php-cs-fixer development dependency, so there was an expectation that it would be installed globally
  2. No path was set on the format command

I originally made a PR to address this, but then I realized that the GitHub Actions are set up to use Pint, so I made a change that I think could make contributing easier in the future.

Changes

Add dependency

I added the the laravel/pint dependency:

"laravel/pint": "^1.0"

NOTE: I constrained to version ^1.0 so that it will install a version old enough to support PHP 8.0 to match this package. Installing on PHP >= 8.1 will install the latest version, v1.18.3 as of submitting this.

Configure Laravel Pint

By default, Pint enforces snake_case method names, which will change:

it_throws_an_exception_when_calling_hasPermissionTo_with_an_invalid_type()

to

it_throws_an_exception_when_calling_has_permission_to_with_an_invalid_type()

The actual name of the method being tested is hasPermissionTo, so I added a pint.json with the following:

{
    "preset": "laravel",
    "rules": {
        "php_unit_method_casing": false
    }
}

Update Composer script

The format script is now defined as:

"format": "pint",

Run Laravel Pint

Finally, I ran composer format and let it run through the project.

Results

The resulting changes are minimal, and all tests are passing. Running composer format should now allow contributors to format their code prior to committing and have the same results as the GitHub Action.

@drbyte
Copy link
Collaborator

drbyte commented Dec 26, 2024

Thanks!

@drbyte drbyte merged commit ee71301 into spatie:main Dec 26, 2024
32 checks passed
@bobbrodie
Copy link
Contributor Author

Oh hey that was quick! Sure thing -- thanks for merging. I know it's small but it was a nice way to settle in :)

@parallels999
Copy link
Contributor

https://github.com/spatie/laravel-permission/blob/main/.github/workflows/fix-php-code-style-issues.yml

Already pint auto commit action

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