Skip to content
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

require puli cli to work out of the box #39

Merged
merged 1 commit into from
Jan 4, 2016
Merged

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Jan 4, 2016

as per the discussion in https://github.com/FriendsOfSymfony/FOSHttpCache/pull/257/files#r48696611

for discovery to work, we need puli. discovery is all about ease of use, mostly a first impression thing (to properly configure your client, you want to instantiate/configure the client yourself). the first impression should be as smooth as possible. puli can't be expected to be globally installed in most systems. the worst that can happen here is that we unnecessarily install the puli cli in vendor. that seems pretty acceptable as worst case...

@ddeboer
Copy link
Contributor

ddeboer commented Jan 4, 2016

👍

sagikazarmark added a commit that referenced this pull request Jan 4, 2016
require puli cli to work out of the box
@sagikazarmark sagikazarmark merged commit 521ac1e into master Jan 4, 2016
@sagikazarmark sagikazarmark deleted the puli-cli branch January 4, 2016 10:38
@Nyholm
Copy link
Member

Nyholm commented Jan 4, 2016

I agree with this merge, but I want to state the implications of this. Since Puli/cli has dependencies on symfony/components: ~2.3 we can not use discovery in a Symfony 3.0 application.

@sagikazarmark
Copy link
Member

Should be fixed in a future version. Any suggestions?

@ddeboer
Copy link
Contributor

ddeboer commented Jan 4, 2016

@Nyholm Yeah, fixed in master.

Let’s just wait with tagging Discovery stable until Puli is.

@Nyholm
Copy link
Member

Nyholm commented Jan 4, 2016

@ddroer yeah they just merged my PR.

Yes. All dependencies must be stable until we can tag a stable release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants