8p / EightPointsGuzzleBundle

⛽️ Integrates Guzzle 6.x, a PHP HTTP Client, into Symfony
MIT License
440 stars 71 forks source link

Review dependencies #173

Closed gregurco closed 6 years ago

gregurco commented 6 years ago
Q A
Bug fix no
New feature no
BC breaks no
Deprecations no
Tests pass yes
Fixed tickets #172
License MIT

What I did:

Merry Christmas :christmas_tree: :snowflake: :slightly_smiling_face:

florianpreusner commented 6 years ago

I'm not sure if it's a good idea to remove dependencies cause they are provided by another one. What happens if symfony/framework-bundle changes that? Than our bundle is not able to work anymore. What's your opinion, @gregurco? I would suggest to add all dependencies even if they are provided by another lib. So we can also make sure to use specific versions, if required.

gregurco commented 6 years ago

@florianpreusner usually when I have such a question, I open packagist, search for top bundles and watch examples from them :slightly_smiling_face: I observed 2 approaches: with all dependencies listed (friendsofsymfony/rest-bundle) and with symfony/framework-bundle + some additional packages not related to the first one (nelmio/api-doc-bundle, jms/serializer-bundle and many others). I think that framework-bundle is trustable enough and composer.json became more clean but yep, we can add all dependencies like fos team did. What do you think? Do we trust framework-bundle or not? :smile:

florianpreusner commented 6 years ago

Well, I do trust framework-bundle but it's more a concept for me. When we decide to remove dependencies in our bundle because they are defined in framework-bundle and the only reason is, that it's a official symfony bundle than I would suggest to keep the dependencies in composer.json. Cause that would be a strange rule, or?

I mean, if every dependency is really following semantic versioning there should be no problem.

florianpreusner commented 6 years ago

If I would start a new plain php project I would define all dependencies that I use. Just to make sure that I have specific versions. And maybe because I do not trust every dependency 😆 .

I can live with both ideas. You have done a lot in the past, @gregurco. And you are a clean coder. So it's up to you.

gregurco commented 6 years ago

@florianpreusner let's try to merge this version of composer.json. We will be in the majority with this mode of dependencies and guess symfony core team takes care of FrameworkBundle and doesn't want to break the whole symfony ecosystem :slightly_smiling_face: And I think we can do new release when will be sure in all changes, because there are many changes not released yet :slightly_smiling_face:

florianpreusner commented 6 years ago

Released in v7.3.0 (https://github.com/8p/EightPointsGuzzleBundle/releases/tag/7.3.0)