Skip to content

Refactor to PHP 8.1#5060

Closed
pxpm wants to merge 5 commits into
v6from
rector-num
Closed

Refactor to PHP 8.1#5060
pxpm wants to merge 5 commits into
v6from
rector-num

Conversation

@pxpm
Copy link
Copy Markdown
Contributor

@pxpm pxpm commented May 5, 2023

This is a Rector run with PHP 8.1 as the target version, and some more changes I did myself that rector didn't get.
(Rector does not automatically delete notated parameters in docblock if they have adicional comments, so there is still ALOT of them!)

Some changes are preferences that we can configure if required.

pxpm and others added 2 commits May 5, 2023 20:04
@tabacitu tabacitu changed the title Refactor rto PHP 8.1 Refactor to PHP 8.1 May 8, 2023
@tabacitu
Copy link
Copy Markdown
Member

tabacitu commented May 8, 2023

Is this ready for me to review? No breaking changes?

@pxpm
Copy link
Copy Markdown
Contributor Author

pxpm commented May 8, 2023

This has no BCs that I am aware of (at least it shouldn't). I would probably just be cautious on the JSON_THROW_ON_ERROR as it may have different results than the previous behavior where errors where not handled.

Apart from that it seems that's just adding a bunch of arrow functions and removing parameters from doc block.

Can it break ? There is the possibility, it's not 0! If we say in doc block @param string $test and we move that string $test to the function, and in reality the function accepts string|null ... yes, it will fail.

I caught some of those as you can see in my commits after the big rector one.

@tabacitu
Copy link
Copy Markdown
Member

tabacitu commented May 8, 2023

Ok got it. I've tagged this as a SHOULD and will take a look as soon as we're done with all MUSTs.

@pxpm pxpm closed this May 20, 2026
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in Backpack v6.x (July 2023-Feb 2024) May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants