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

Apply PHP 8.0 Syntax and constructor promotion #140

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

samsonasik
Copy link
Member

Signed-off-by: Abdul Malik Ikhsan samsonasik@gmail.com

Q A
QA yes

Description

Since composer.json require php 8.0, php 8.0 syntax can constuctor promotion can be applied.

Signed-off-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
@Ocramius Ocramius added this to the 3.5.0 milestone Oct 21, 2022
@samsonasik
Copy link
Member Author

All green 🎉

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @samsonasik!

if ($first instanceof ContainerInterface) {
$container = $first;
$instance = $second;
} else {
$container = $second;
$instance = $first;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like that rector gets rid of random whitespace, but it's not critical :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's on PHP-Parser latest dev-master for keep the space, but probably only will land on PHP-Parser 5.0 nikic/PHP-Parser#891

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@Ocramius Ocramius self-assigned this Oct 21, 2022
@Ocramius Ocramius merged commit 111e08a into laminas:3.5.x Oct 21, 2022
@Ocramius
Copy link
Member

Thanks @samsonasik!

@samsonasik samsonasik deleted the apply-php80 branch October 21, 2022 14:21
@TomasVotruba
Copy link

Amazing work 🥳

@Ocramius
Copy link
Member

@TomasVotruba we still need to automate @samsonasik away xD

@TomasVotruba
Copy link

@Ocramius We're working on that together... 😉 You'll notice our Github accounts start to look like human speaking 😆

@Ocramius
Copy link
Member

@TomasVotruba aware, just saying that we probably need to put some effort in making this some sort of CI / bot.

Absolutely hats off to @samsonasik's thankless and constant efforts, but I think that would be a big win for the ecosystem at large 👍

@TomasVotruba
Copy link

TomasVotruba commented Oct 24, 2022

@Ocramius We already use it in CI as automated contributor: https://github.com/rectorphp/rector-src/blob/main/.github/workflows/rector.yaml
Just change the rector.php setup and everything else happens by itself :)

@Ocramius
Copy link
Member

Yeh, I discussed it with @samsonasik before: what we need is something that suggests changes (with github actions output format), rather than something that applies the changes :)

I haven't worked on it myself yet, but probably something for https://github.com/rectorphp/rector/tree/b9c04e2a17bf233037d76ea2b9b2d7b411b5ec41/src/Console/Formatter

Someday I'll try helping out there :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants