Skip to content

Updated scripts in composer.json#41

Merged
driehle merged 1 commit intodoctrine:2.2.xfrom
driehle:feature/static-analysis
Nov 5, 2021
Merged

Updated scripts in composer.json#41
driehle merged 1 commit intodoctrine:2.2.xfrom
driehle:feature/static-analysis

Conversation

@driehle
Copy link
Member

@driehle driehle commented Nov 4, 2021

No description provided.

@driehle driehle requested a review from greg0ire November 4, 2021 15:40
@driehle driehle self-assigned this Nov 4, 2021
@driehle driehle added the Enhancement New feature or request label Nov 4, 2021
@driehle driehle added this to the 2.2.1 milestone Nov 4, 2021
@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2021

Please improve your commit message according to the contributing guide. In the present case, I have no idea why you would want to move psalm.xml to psalm.xml.dist

@driehle driehle force-pushed the feature/static-analysis branch from b93514e to 04653e0 Compare November 4, 2021 16:25
@driehle
Copy link
Member Author

driehle commented Nov 4, 2021

@greg0ire I considered this as self-explanatory, sorry 🤷‍♂️

I updated the commit message accordingly.

@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2021

No worries. Why would a user want to override psalm.xml.dist?

@driehle
Copy link
Member Author

driehle commented Nov 4, 2021

We are doing the same for psalm.xml.dist and phpunit.xml.dist (accross all repositories, I think). Though I guess that most users (I am actually refering to developers here, not end users) won't override these files, there may be circumstances where it is helpful to override them. The same principle applies to psalm.xml.dist. We should probably rename phpstan.neon to phpstan.neon.dist as well though.

@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2021

I can think of some reasons to do it for phpunit.xml on some repositories, but I'd be really interested to know how many people actually resort to it, or why you would need to do it in the case of Psalm.

I think it only complicates things as one might forget they once did it and wonder why they get different results locally or in the CI. Personally, the very rare times I've had to use a different database, I've instead run vendor/bin/phpunit -c ci/github/phpunit/oci8.xml.

@driehle
Copy link
Member Author

driehle commented Nov 5, 2021

The only reason I could think of regarding Psalm is lowering the level locally to see the issues that should probably targeted in a next major. Anyways, I don't care that much, my idea was rather to have all build tools (i.e. PhpCS, PHPUnit, PHPStan and Psalm) consistent in terms of their configuration.

@driehle driehle force-pushed the feature/static-analysis branch from 04653e0 to 678021e Compare November 5, 2021 09:24
@driehle driehle changed the title Moved psalm.xml to psalm.xml.dist and updated scripts in composer.json Updated scripts in composer.json Nov 5, 2021
@driehle
Copy link
Member Author

driehle commented Nov 5, 2021

@greg0ire I updated the commit, let's keep psalm.xml for now.

@driehle driehle merged commit 91bb599 into doctrine:2.2.x Nov 5, 2021
@driehle driehle deleted the feature/static-analysis branch November 5, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants