Skip to content

Explicitly declare dependencies in composer.json #10

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

Closed

Conversation

Matthimatiker
Copy link
Contributor

The HttpKernel bridge requires the php-pm/php-pm component to work.
This change declares the mentioned dependency explicitly. Therefore, the development of the bridge is simplified as a composer install is enough to get started.

@andig
Copy link
Contributor

andig commented Sep 2, 2015

*@dev or dev-master? I had intentionally not done so as requirig a package that itself requires dev packes imho may cause troubles with minimum-stability. Or is that assumption wrong?

@Matthimatiker
Copy link
Contributor Author

Indeed, this may lead to problems. I would also prefer a reference to a stable version, but at the moment there are no tagged versions.
I guess the added dependency just makes the current state clear, at least not worse than before.

I used *@dev instead of dev-master as I assume that this reference does not restrict php-pm to a specific branch.

@andig
Copy link
Contributor

andig commented Sep 2, 2015

As long as we don't have stable version I'd suggest leave it as is. Much easier to spot a missing dependency than trying to troubleshoot composer's error messages.

As for *@dev I must say I don't know- never seen that before.

@Matthimatiker
Copy link
Contributor Author

The problem with the current state is, that you do not get a working library after composer install.
For example, this prevents one from writing unit tests as required classes are missing.

An alternative could be to add php-pm/php-pm as dev requirement. This does not affect root packages that include this library, but it provides a working version, when the library is used standalone (during development).

@marcj
Copy link
Member

marcj commented Feb 17, 2016

As dev requirement is here the way to go.

@andig
Copy link
Contributor

andig commented Mar 16, 2016

As dev requirement is here the way to go.

@marcj you've recently mentioned production deployment. How about declaring the current status to be a 1.0.0 across the packages and then take it from there?

@marcj
Copy link
Member

marcj commented Mar 16, 2016

yes i wanted to do it, but doing currently some improvements which I want to include in the first version😊 In the next days this is happening

@andig
Copy link
Contributor

andig commented Mar 16, 2016

Nice. I think we shouldn't merge anything else until then.

@marcj
Copy link
Member

marcj commented Mar 18, 2016

Alright, I'm going to close this then and will adjust the composer.json straight with other changes. Thanks anyway guys!

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.

3 participants