Skip to content

[Downgrade] Composer's platform check must use the PHP version below#4879

Merged
TomasVotruba merged 2 commits intomasterfrom
downgrade-composer-platform-version-check
Dec 14, 2020
Merged

[Downgrade] Composer's platform check must use the PHP version below#4879
TomasVotruba merged 2 commits intomasterfrom
downgrade-composer-platform-version-check

Conversation

@leoloso
Copy link
Copy Markdown
Contributor

@leoloso leoloso commented Dec 14, 2020

When using set downgrading-php80, we're downgrading from PHP 8.0 to its previous version, i.e. 7.4. Hence, Composer's platform check (on vendor/composer/platform_check.php) must do if (!(PHP_VERSION_ID >= 70400)) { (not 80000).

Fixed for all downgrade sets.

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 14, 2020

@simivar all these unsuitable changes were applied automatically by Rector Rectify: 182aef1

That's why this is happening in all my PRs!

If this is bug, I can create a new issue about it...

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Dec 14, 2020

If this is bug, I can create a new issue about it...

It is indeed unrleated to PR, but we should fix it for next PRs.
Please create failing test case for the broken rule

@TomasVotruba TomasVotruba merged commit b8b86de into master Dec 14, 2020
@TomasVotruba TomasVotruba deleted the downgrade-composer-platform-version-check branch December 14, 2020 16:29
@TomasVotruba
Copy link
Copy Markdown
Member

Thank you

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 15, 2020

Please create failing test case for the broken rule

There's no broken rule. It's just Rector Rectify working properly, on 1 file (for rule PropertyAccessorCreationBooleanToFlagsRector) that, somehow, had not been previously rectified...

I guess when this happens again, we can switch to master, and do some simple change just to trigger Rector Rectify, and commit the rectified code so it doesn't spill over the other PRs

@TomasVotruba
Copy link
Copy Markdown
Member

The rule can do better. It should be fixed now, because it changes our code for worse

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 15, 2020

Ok, hopefully I get it now. The problem is that if there are 2 or more variables ($magicGet and $magicSet) that can be renamed to the same name (as $classConstFetch), then the rule should be skipped...

Then, $flags renamed as $bitwiseOr, that's OK.

Is this right?

@TomasVotruba
Copy link
Copy Markdown
Member

Yes, exactly.

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 15, 2020

Added now in #4898. But I wonder if I got it right? The tests are failing in a different way to Rector Rectify's behavior...

TomasVotruba added a commit that referenced this pull request Aug 29, 2023
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