search

cakephp / upgrade

Upgrade tools for CakePHP meant to facilitate migrating from one version of the framework to another
MIT License
110 stars 60 forks source link

Add more 5.x upgrades #265

Closed dereuromark closed 9 months ago

dereuromark commented 9 months ago

Removed the command rector, as it has negative side effects, tons of issues like

  40     PHPDoc tag @return with type int|void|null is not subtype of native type int|null.                           
  44     Method App\Command\EventnumberCommand::execute() should return int|null but return statement is missing.

which wouldnt need to be there

If someone wanted this, they could add this as "strict rule" on top themselves I guess. They should only be applied if the docblock type is int|null, not if it is int|null|void.

LordSimal commented 9 months ago

A test showcasing that this works would be nice... Usually you only need to adjust the already existing files in the https://github.com/cakephp/upgrade/tree/5.x/tests/test_apps/original/RectorCommand-testApply50/src directory + the corresponding upgraded directory

dereuromark commented 9 months ago

I ran them on live code, they seem to work fine. But sure, someone could add tests here. Just wanted to log them here so they are not forgotten.

dereuromark commented 9 months ago

I am confused, why is phpunit not installed/available? I see it needs make install-dev Any reason we cannot use normal process here using require-dev?

LordSimal commented 9 months ago

Dependency problems with the app's autoloader since performing the upgrade loads both the app's autloader as well as the upgrade tool's autoloader. We already had this conversation in the past and a built phar file would resolve this as far as I remember but no one wanted to look into how to build phar files via Github CI