Pulled tools used internally by library out of main composer.json

FelixJacobi commented 2 years ago

This no longer enforces consumers to install our internal development dependencies when installing the library in development mode. It also has the benefit, that the tools dependencies no longer can conflict with our main composer.json.

As far as I can see, everything still works like it worked before,

Technical details:

Each tool has its own composer.json
Each tool has a shell wrapper which installs the tool via composer on demand
The shell wrapper calls the original tool at the end.

The only difference is that instead of vendor/bin/<tool> tools/<tool> must be used.

Blocked by #115 and base for updating CS fixer and PHPUnit.

SamuelWei commented 2 years ago

I like the idea, but haven't tested it yet. This (#116), #114 and #115 could be released as v5.0.0.

FelixJacobi commented 2 years ago

This (https://github.com/littleredbutton/bigbluebutton-api-php/pull/116), https://github.com/littleredbutton/bigbluebutton-api-php/issues/114 and https://github.com/littleredbutton/bigbluebutton-api-php/pull/115 could be released as v5.0.0.

Would be more a 4.3 for me except for PHP 8.1 as no major BC breaks contained otherwise. We should strictly follow Semantic Versioning 2.0.0 | Semantic Versioning here to make things predictable for library consumers.

sualko commented 2 years ago

For me personally this pr makes everything harder to read.

This no longer enforces consumers to install our internal development dependencies when installing the library in development mode

If we merge this pr it will result in unpredictable behavior in my view. That's how composer works and we should not change that. If you are in dev mode, all dev dependencies should be installed. How are other libraries are handling such cases?

FelixJacobi commented 2 years ago

If we merge this pr it will result in unpredictable behavior in my view.

Hm, I guess, you need to explain me your point here further. What exactly is more unpredictable than before?

That's how composer works and we should not change that

Yeah, but that does not fit well for libraries. Libraries need to be usable as flexible as possible. The dev vs. non-dev in my opinion is only suitable for the "end-user applications". A problem I am having with the library currently: I cannot upgrade to PHPUnit 9 in my project, as we're locking it down to 8 here. For my application, I don't care what PHPUnit version littleredbutton is using. I won't execute the tests of the library (which are also not shipped). Also, we have tools here, which are completely internal, like PHP CS Fixer and Psalm. We're only using them for our CI here, so that is not a real code dependency at all.

If you are in dev mode, all dev dependencies should be installed.

Yeah, but that are not my dev dependencies from the consuming application perspective.

How are other libraries are handling such cases?

An alternative, which is very often used, is phar-io/phive: The Phar Installation and Verification Environment (PHIVE). I have experiences with it.

Pro: It downloads single phar files, very simple.

Cons:

It uses GitHub release pages as source. This always breaks after a few weeks, when used tool version moves to page 2 (yep).
phar files are precompiled archives, so cannot install Plugins e.g. for PHPUnit or Psalm

This almost adapts the behavior of phive, but uses the powers of composer. Unless you need to update tools, you don't need to touch very much. You can simply use it as always, only the paths changed.

Symfony used an almost identical way for Psalm: https://github.com/symfony/symfony/blob/6.1/.github/workflows/psalm.yml

The only difference to there, is that here is no executable for you to run the tools locally, which is better for DX in my opinion.

sualko commented 2 years ago

What exactly is more unpredictable than before?

It probably depends on the definition of "dev dependencies", but your post makes it clearer to me. If this pr makes the use of this library easier, I can life with the more complicated dev env. I just would like to see some kind of documentation which explains the step. Maybe a readme in the tools folder or something.

FelixJacobi commented 2 years ago

Sure. I can add this and explain the motivation like already done here ^^ .

FelixJacobi commented 2 years ago

@sualko README added.

littleredbutton / bigbluebutton-api-php

Pulled tools used internally by library out of main composer.json #116