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

Add phpstan command #53

Closed
wants to merge 11 commits into from
Closed

Add phpstan command #53

wants to merge 11 commits into from

Conversation

jameswilson
Copy link

The Issue

This is a second attempt at a fix for #13.

Pulling in work from and building on top of PR #14 using methodology described here: https://stackoverflow.com/a/39807499/413538

How This PR Solves The Issue

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@jameswilson jameswilson mentioned this pull request May 22, 2024
@weitzman
Copy link
Collaborator

The problem with expand-composer-json in this case is that it edits the project's composer.json (thats what makes it work). This causes a diff for the user which she might commit. Thats not the worst thing, but its not the best either. When we use ddev expand-composer-json , we always use a separate composer.json file and we delete it afterwards.

All in all, I prefer to wait for phpstan-drupal to adopt the drupal-finder - mglaman/phpstan-drupal#757

For a workaround, projects are welcome to populate the composer.json.extra.installer-paths so that drupal-finder works and this vendor/bin/phpstan works

@jameswilson
Copy link
Author

jameswilson commented May 23, 2024

The problem with expand-composer-json in this case is that it edits the project's composer.json (thats what makes it work). This causes a diff for the user which she might commit. Thats not the worst thing, but its not the best either. When we use ddev expand-composer-json , we always use a separate composer.json file and we delete it afterwards.

The method used in the command doesn't edit composer.json, it exports a custom $COMPOSER environment variable first, and therefore only adds untracked composer.contrib.json to the repo root, which can be easily cleaned up. This is exactly how the ddev poser command works too.

For a workaround, projects are welcome to populate the composer.json.extra.installer-paths so that drupal-finder works and this vendor/bin/phpstan works

I considered that but didnt want to dirty up composer.json with needless settings that were ddev-specific. I prefer an approach that leaves no trace on existing files, and is self-contained inside the .ddev/ folder, whenever possible. This PR does that.

@weitzman
Copy link
Collaborator

dupe of #14

@weitzman weitzman closed this Jul 22, 2024
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