Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed the plugin for use in sylius 1.9 - Hotfix #252

Closed
wants to merge 1 commit into from
Closed

fixed the plugin for use in sylius 1.9 - Hotfix #252

wants to merge 1 commit into from

Conversation

debugteam
Copy link

@debugteam debugteam commented Mar 25, 2021

Q A
Bug fix? yes
New feature? no
BC breaks? no
Fixed tickets ?

made a quick debugging session after installing the plugin on a sylius 1.9 and saw it wasn't working. Instead of learning how the architecture works i just created a decorator around the givven entityRepository to add getImagetype functionality to the repository object and by putting an emtpy string to the initialisation of treebuilder i suppressed an error..

I did a testinstall made a test export and import but i don't recommend this change for the long run and will definatly improve it as soon as i spend more time with the code.

…e injection of entityRepository to something specific rather than using a decorator to fix that
@debugteam
Copy link
Author

debugteam commented Mar 25, 2021

checks have failed... i have asked about it in the dev chat of sylius on why they didn't use 2.0 instead of 1.9 as new version, since it breaks something. They answered with:

Hey "@jschultz", it’s very sad to hear that. But keep in mind, that you can expect from us what is written in our Backward Compatibility Promise. If you found something, that we broke, but we shouldn’t please raise an issue. A lot of these changes are forced by changes in external libraries and not doing them would slow down the ecosystem. Still, being sad to said that, but this is the trade-off that we’ve decided to do. What is more, version 1.4 has reached EOL over a year ago(according to our Release Cycle). This plugin should be upgraded at least to v1.8.

@debugteam
Copy link
Author

Also i personally recommend upgrading to php 7.4.
PHP 7.2 has strong issues and shouldn't be used anymore!

@lsmith77
Copy link
Collaborator

thank you .. we will likely merge #251 sometime this week and make a new release. if there is some hold up there, we may consider a hotfix.

@debugteam
Copy link
Author

Didn't check for PRs... but i guess i also recommend merging #251 instead of my 15 minute patch... Trying a new shop platform with a new customer on a video chat.. and then havving to fix the product import is a quiet challenging task... mastered it somehow and thought i share..

@lsmith77
Copy link
Collaborator

no longer needed as #251 was merged. thx for the contribution however!

@lsmith77 lsmith77 closed this Mar 27, 2021
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