search

m-vo / contao-facebook-import

Facebook posts and events for Contao Open Source CMS
Other
11 stars 5 forks source link

Use nickdnk/graph-sdk #34

Closed rabauss closed 3 years ago

rabauss commented 3 years ago

The fork from nickdnk is now the most downloaded php8 graph-sdk: https://packagist.org/?query=facebook%20graph%20php8 And pdir/social-feed-bundle is also using it - https://github.com/pdir/social-feed-bundle/commit/1b2342fd23e43a10d70d3e76e9a5af45eb47bb49#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34

m-vo commented 3 years ago

:thinking: I think I'd rather go with a suggestion for this bundle in the composer.json and a modified hint in the README.md. Although it's the most downloaded for now, I'm not confident this a long(er) term replacement.

Any other feedback on this?

rabauss commented 3 years ago

So there are only two solutions:

  1. keep the unsupported requirement facebook/graph-sdk and you need to add a fork e.g. brainexe/php-graph-sdk for php8 like suggested now
  2. require the nickdnk/graph-sdk so you can install the bundle with php8 without trouble. the graph-sdk is not supported anymore by facebook!

By the way you cannot prefer to use the renamed nickdnk/graph-sdk, which is a fork of the suggested brainexe/php-graph-sdk as long as this bundle requires facebook/graph-sdk

fritzmg commented 3 years ago

May be you should open a PR at nickdnk/graph-sdk to add a replace directive to the composer.json. Then it would not matter.

rabauss commented 3 years ago

You're absolutely right - thanks @fritzmg! After my PR is merged I will update this PR because than we should only need a hint in the Readme, that you can require nickdnk/graph-sdk

rabauss commented 3 years ago

I updated the pull request because you can require now nickdnk/graph-sdk as replacement for facebook/graph-sdk. The only downside is that you have to install nickdnk/graph-sdk first before installing mvo/contao-facebook-import on php 8.

Should we better mention this in the readme or add it as step 0?

m-vo commented 3 years ago

Thank you @rabauss.