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

TASK: Add missing repositories/distributionPackages configuration to composer #9

Merged
merged 1 commit into from Jan 2, 2019

Conversation

Projects
None yet
3 participants
@daniellienert
Copy link
Member

daniellienert commented Dec 14, 2018

On a fresh installed base distribution, the repositories section is missing. ./flow package:create depends blindly on that key and throws an exception.

Resolves: neos/flow-development-collection#1489

@daniellienert daniellienert requested review from mficzel and kdambekalns Dec 14, 2018

@mficzel
Copy link
Member

mficzel left a comment

Fine by reading and makes sense for flow-base-dist.

However i would consider reading this key blindly also an error and suggest to fix it there aswell.

@@ -26,6 +26,12 @@
"phpunit/phpunit": "~6.0.0",
"mikey179/vfsstream": "~1.6"
},
"repositories": {
"distributionPackages": {

This comment has been minimized.

@kdambekalns

kdambekalns Dec 14, 2018

Member

Wait… why give it a name at all? I never did that, so far.

This comment has been minimized.

@kdambekalns

kdambekalns Dec 14, 2018

Member

Also, using JSON object notation (not an array) does not guarantee order. See https://getcomposer.org/doc/04-schema.md#repositories

@@ -26,6 +26,12 @@
"phpunit/phpunit": "~6.0.0",
"mikey179/vfsstream": "~1.6"
},
"repositories": {
"distributionPackages": {

This comment has been minimized.

@kdambekalns

kdambekalns Dec 14, 2018

Member

Also, using JSON object notation (not an array) does not guarantee order. See https://getcomposer.org/doc/04-schema.md#repositories

@mficzel

This comment has been minimized.

Copy link
Member

mficzel commented Dec 14, 2018

@kdambekalns this is what we do aswell as in the neos-base-distribution. Works fine and naming things is not bad. Composer handles both fine and is also allowed by the spec. Also if you need an order between your local repositories you are in a situation where a single package is available via multiple repos. I would consider this very bad practice anyways.

@kdambekalns

This comment has been minimized.

Copy link
Member

kdambekalns commented Dec 14, 2018

Fine, but again:

However, JSON key/value pairs are to be considered unordered so consistent behaviour cannot be guaranteed.

I'd not do this.

@daniellienert

This comment has been minimized.

Copy link
Member Author

daniellienert commented Dec 15, 2018

Hey @kdambekalns,
In fact that is how a composer config distributionPackages path "./DistributionPackages/*" would generate the config.
As @mficzel already mentioned, I just copied the lines from the Neos base distribution.
I am absolutely fine with the array variant, if you are against the object. But the Neos distribution should then be updated as well.

@kdambekalns

This comment has been minimized.

Copy link
Member

kdambekalns commented Dec 19, 2018

Wow, now that is stupid^Wweird–warning about order issues but creating an object when using the config command.

@daniellienert daniellienert merged commit 4b5857f into 5.2 Jan 2, 2019

@daniellienert daniellienert deleted the daniellienert-patch-1 branch Jan 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment