Brille24 / SyliusCustomOptionsPlugin

A Sylius plugin that adds customer options
MIT License
47 stars 34 forks source link

Update doctrine configuration #149

Closed JoppeDC closed 7 months ago

JoppeDC commented 7 months ago

This PR References issue https://github.com/Brille24/SyliusCustomOptionsPlugin/issues/148

Note: The overwriting of the Sylius resources has also been removed. The test application should probably be updated to reflect the newer way of working

Feedback is appreciated

mamazu commented 7 months ago

The PR looks good so far. One thing that we should probably mention in the docs/upgrade guide.

I am not sure what you mean with the "newer way of working".

JoppeDC commented 7 months ago

I have updated the Readme to reflect the changes in regards to migrations. I've also added a small UPGRADE doc, documenting the changes to the migrations and ORM mapping.

Please feel free to let me know if you have any more changes you'd like to see!

mamazu commented 7 months ago

Perfect. I think this would probably be tagged as 4.0 then as this is a BC break.

I'm not sure if that's going to play well with other projects still using XML (and wanting to use this package). Is this possible? I'll do some testing in our Sylius project to see if it still works.

JoppeDC commented 7 months ago

Unfortunatly i do not currently have a project that uses XML mapping, although this should not be a problem.

mamazu commented 7 months ago

Anything else that's needs to be done? Otherwise I'd merge it and tag it as the new version.

JoppeDC commented 7 months ago

I'm currently using it pinned on my fork branch, and during my testing it has been working fine.

Maybe a rename of the UPGRADE markdown file based on the tag you're planning to push?

mamazu commented 7 months ago

Sure, that would be great. It's planned as 4.0

JoppeDC commented 7 months ago

Awesome, i've updated the readme @mamazu

mamazu commented 7 months ago

Thanks for your contribution. :tada: