[PR #8144] Automated fixes with phpcbf + manual fixes #10826

Open
opened 2026-01-22 16:08:49 +01:00 by admin · 0 comments
Owner

Original Pull Request: https://github.com/doctrine/orm/pull/8144

State: closed
Merged: Yes


Blocked by https://github.com/doctrine/orm/pull/8150

I often tell people to run vendor/bin/phpcbf to fix their PRs, by habit,
which sometimes results in confusion because it produces a huge unwanted diff.

Also, and this is key, this reduces greatly the diff between branches, which should mean far fewer conflicts when merging up, or when rebasing between 2.7. and 2.8, so maybe this will make @guilhermeblanco 's work easier as per https://github.com/orgs/doctrine/teams/doctrinecore/discussions/14 . Once this is merged, I think we should vow never to change the CS config unless we do it from the lowest branch and merge up.

There are still a lot of cs issues that need to be fixed before we can switch from git-phpcs to just phpcs.

The rules that required the most work are rules about specifying the types of elements instead of just specifying array. I thought that we could ignore it, but then that would mean that it wouldn't be required for new code, which would be bad, so I tried to describe it when I could figure out easily and fast enough, and in other cases, I just used mixed[]. These kind of changes are in the third commit, and if you are unhappy with that, I can drop that commit that has manual fixes. That will still leave us with a vendor/bin/phpcbf that runs without errors.

Next steps, before merging this: creating merge up PRs, so that they can be pushed atomically all at once. That way we have no unknowns about the merge up and how hard they are. I gave merging into 2.8.x a quick try and there are not that many files conflicting.

I still would like a review, or at least comments about the above plan before going ahead, because this took me a lot of time, so I'd like to have everyone on board before I invest even more time on this.

**Original Pull Request:** https://github.com/doctrine/orm/pull/8144 **State:** closed **Merged:** Yes --- ~Blocked by https://github.com/doctrine/orm/pull/8150~ I often tell people to run `vendor/bin/phpcbf` to fix their PRs, by habit, which sometimes results in confusion because it produces a huge unwanted diff. Also, and this is key, this reduces greatly the diff between branches, which should mean far fewer conflicts when merging up, or when rebasing between 2.7. and 2.8, so maybe this will make @guilhermeblanco 's work easier as per https://github.com/orgs/doctrine/teams/doctrinecore/discussions/14 . Once this is merged, I think we should vow never to change the CS config unless we do it from the lowest branch and merge up. There are still a lot of cs issues that need to be fixed before we can switch from git-phpcs to just phpcs. The rules that required the most work are rules about specifying the types of elements instead of just specifying `array`. I thought that we could ignore it, but then that would mean that it wouldn't be required for new code, which would be bad, so I tried to describe it when I could figure out easily and fast enough, and in other cases, I just used `mixed[]`. These kind of changes are in the third commit, and if you are unhappy with that, I can drop that commit that has manual fixes. That will still leave us with a `vendor/bin/phpcbf` that runs without errors. Next steps, before merging this: creating merge up PRs, so that they can be pushed atomically all at once. That way we have no unknowns about the merge up and how hard they are. I gave merging into 2.8.x a quick try and there are not that many files conflicting. I still would like a review, or at least comments about the above plan before going ahead, because this took me a lot of time, so I'd like to have everyone on board before I invest even more time on this.
admin added the pull-request label 2026-01-22 16:08:49 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#10826