silverstripe / silverstripe-populate

Populate your database through YAML files
BSD 3-Clause "New" or "Revised" License
26 stars 24 forks source link

Updating to work with SSv5 #51

Closed catharsisjelly closed 1 year ago

catharsisjelly commented 1 year ago

Making SS v5 compatible

Release wise I suggest you do what silverstripe/versioned did and mark this as a major (breaking) change

chrispenny commented 1 year ago

Thanks for kicking this off, @catharsisjelly!

Update workflow

I updated the Github workflow to match SS 5 requirements (that being, we rely on the default matrix).

Updated method signatures

Now is our opportunity to update method signatures to be more strict.

Regarding the $class argument in PopulateFactory::createObject()

I did prefer it being renamed to $class (from $name on the parent class), as that was more contextually descriptive, but because PHP 8 now supports named arguments, I thought this might cause some issues if there is inconsistency between the parent and child class.

Testing

Just a heads up that I have not yet had the opportunity to manually test these changes.

catharsisjelly commented 1 year ago

Thanks for kicking this off, @catharsisjelly!

No problem!

Testing

Just a heads up that I have not yet had the opportunity to manually test these changes.

I have an SS5. Site I'm using this for at the moment so I will pull these changes and let you know if anything breaks.

catharsisjelly commented 1 year ago

Working fine for me, happy to try a few things out if you need it.