Errors with RockMigrationsConstants Traits

[gebeer]

Hi @bernhard,

I already fell in love with the new config migrations feature in RockMigrations > 6.0.0. I am currently on v6.0.1

So much cleaner to have migrations organized in this way.

I just tried to implement the part about the constant traits, described at https://www.baumrock.com/en/processwire/modules/rockmigrations/docs/config-migrations/#class-constant-traits

First it would not work at all as described in the docs. I noticed that my file at site/modules/Site/RockMigrations/RockMigrationsConstants.php never got picked up because of searching for those files did not recurse into module subdirectries. Fixed in PR#70 commit  https://github.com/baumrock/RockMigrations/pull/70/commits/faba5475a008ab14415c0e35f2c5c78d2e91f0b1.

First problem:

After fixing that, I get the Compile Error
Traits cannot have constants

Actually, constants are only allowed in Traits since PHP 8.2.

So this would change the module dependencies from PHP>=8.0 to PHP>=8.2

I don't think this was intended. But it should be updated or at least documented that the automated constants thingy requires PHP>=8.2.

Second problem:

After upgrading to PHP8.2 I ran into the next issue:

This is caused by the constant name "template_basic-page" containing a hyphen. As it seems, they are not allowed in constant names in PHP at all (https://www.php.net/manual/en/language.constants.php):

Quote

The name of a constant follows the same rules as any label in PHP. A valid constant name starts with a letter or underscore, followed by any number of letters, numbers, or underscores. As a regular expression, it would be expressed thusly: ^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$

For years I have naming conventions for templates and fields in PW. Template names always contain hyphens like in "basic-page". Field names always contain underscores like in "my_field". This is also helpful when looking at code to discern templates from fields on the first glance.

I fixed this by replacing - with _ in constant names. See https://github.com/baumrock/RockMigrations/pull/70/commits/a06c6062c61b5264932a8044cf25bac0c689f8cf

The issue with PHP8.2 still remains to be resolved. I suggest adding the requirement only to docs  about RockMigrationsConstants.php because other than that RockMigrations seemed to work fine on 8.1 in my tests.

 

[bernhard]

Hey @gebeer thanks for trying out Config Migrations so early 🙂 💪

10 hours ago, gebeer said:

I already fell in love with the new config migrations feature in RockMigrations > 6.0.0.

Same here 😍 

10 hours ago, gebeer said:

First it would not work at all as described in the docs. I noticed that my file at site/modules/Site/RockMigrations/RockMigrationsConstants.php never got picked up because of searching for those files did not recurse into module subdirectries. Fixed in PR#70

The file should not be in /site/modules/Site/RockMigrations/... but should live side by side with Site.module.php! I just had a look into the docs and you are right - the instructions there are like you say. I have to look into what you said tomorrow, but if you have time and read this in the meantime, then I'd appreciate if you could try to create the file /site/modules/Site/RockMigrationsConstants.php and see if that maybe fixes some other of your mentioned issues?

10 hours ago, gebeer said:

The issue with PHP8.2 still remains to be resolved. I suggest adding the requirement only to docs  about RockMigrationsConstants.php because other than that RockMigrations seemed to work fine on 8.1 in my tests.

Thx a lot! I'll likely do exactly that 🙂 As of 2 days ago we have PHP8.4 released and PHP8.1 gets security fixes only - so I think it would even be fine to have PHP8.2 as a requirement for the module. Any other votes?