search

fooman / printorderpdf-implementation-m2

5 stars 8 forks source link

Remove generated pdf files according to the core update #3

Closed me2r036 closed 5 years ago

me2r036 commented 5 years ago

Remove generated pdf files according to the core update

fooman commented 5 years ago

@me2r036 Thank you for this pull request. I think it's a good change and I would like to include it. Unfortunately reading the corresponding change in core here I believe that merging your changes mean that the only supported Magento version is 2.3.1 (as opposed to 2.1/2.2/2.3) as we would be calling strlen on an array. Please correct me if that is not the case.

me2r036 commented 5 years ago

@fooman From what I can see the code should work with ^2.2.6. You can see the updated commit here, so the strlen function works on the actual file content rather than an array.

fooman commented 5 years ago

Thanks for looking into this. Would you mind also updating this line to reflect the updated dependencies, ie "magento/framework": "^101.0.6 | ^102.0.0",

Please note I will not yet merge this even with the changes just yet as I still want to maintain 2.1 compatibility. However once I am ready to drop this I can merge this in.

fooman commented 5 years ago

@me2r036 let's merge your PR into this repo with the above changes onto a new branch. That way it should get picked up by packagist and people could already use with composer require fooman/printorderpdf-implementation-m2:dev-feature/remove-generated-pdf-file

fooman commented 5 years ago

Good stuff - thank you!

me2r036 commented 5 years ago

@fooman That's great, thanks :)