-
Notifications
You must be signed in to change notification settings - Fork 61
puli cli is now required by discovery #267
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
Conversation
looks like puli is not working as it should. @sagikazarmark could you please have a look and tell us what we do wrong? |
@webmozart can this be an incompatibility between beta versions? |
The current development build is broken. I recommend to use the beta versions for better stability. The next beta should arrive by mid January. |
Locking |
https://github.com/puli/discovery/blob/1.0.0-beta8/composer.json does not mention symfony so we should be good. |
Where was the problem in the first place? @Nyholm provided a PR for symfony 3.0 support. Should we add the lock back to discovery, or override in FOS HTTP Cache for the time being? |
the symfony 3 problem was with puli/cli. i try to fix stuff here: php-http/discovery#40 i have no idea why composer is ignoring the prefer-stable: true flag - otherwise it should install discovery beta8 and things would not fail here. |
Sorry for the problems guys :( The next beta release is compatible with Symfony 3.0 and should fix a lot of your problems. |
Thanks @webmozart |
puli/discovery is compatible with sf3. It is php-http/discovery that is not compatible with sf3. Because: php-http/discovery => puli/cli => padraic/phar-updater => symfony/finder:~2.2 Here is the PR to fix that issue: humbug/phar-updater#16 |
@webmozart why does puli/cli depend on phar-updater? should that not be a require-dev for when you build the phar? or can the phar also be built out of any project that installed puli/cli into vendor? but even then, should phar-updater not be optional? i guess there are rarely cases where one would want to build the phar out of anything but a clone of the puli/cli repository directly. |
@dbu a phar is meant to be built without dev requirements (you don't want to include PHPUnit in them). So making it a requirement is logical. |
okay. but the phar-updater is irrelevant for the cli when used in vendor. it feels like the phar packaging stuff and some of puli's application logic got mixed together in one repository. we certainly don't need the phar updater here (and it creates problems because of dependencies). |
I guess the phar updater package provides the An alternative solution would be removing the update command from the cli package itself (as it doesn't makes sense there) and create a cli-phar package, require phar-updater there and add the update command there. Not sure though wether it worths the hassle, but if installing cli is a viable option, then we don't need self-update inside that. |
I don't quite understand the point of this discussion. Isn't humbug/phar-updater#16 fixing this issue? |
@dbu the question IMO is why you need to install the CLI in your vendors. The CLI is meant to be its own project AFAIK. |
@stof It can be either. You can either install the PHAR globally or require the CLI in your project. |
@webmozart yes, it does. I think it is just a theoretical debate about the purpose and usage of CLI package, at least indirectly. @stof One reason is that installing cli is easier than saying have it installed in your prefix. I guess, one day, puli will be installed on every developer machine...but this is not that day (yet). |
this is fixing the current problem at hand. but as mark says, i question why we would have that dependency in general. the problem at hand illustrates one of the issues of having an "unnecessary" dependency. phar-updater could one day conflict with something that nothing we or puli use in the non-phar usecase have a conflict with, or random other problems. and its confusing for users of FOSHttpCache why it ends up installing a phar-updater... |
php-http/discovery#41 makes me wonder wheather we really want discovery included by default... its dependencies are indeed pretty massive. |
@dbu IMO, discovery should always be a suggested dependancy when user want a very friendly experience. But for advanced user it does not make any sense IMO. The debate here, is what do you want most ? : something nice with a friendly experience, or something robust with more documentation ? |
One possible way of making discovery optional is adding an additional check if the desired discovery class exists. If not, fall back to exception. |
Let's move the CLI part of this discussion to php-http/discovery#41 |
puli cli is now required by discovery
No description provided.