[Composer] Fixed minimum-stability for 2.1-beta related packages

[alongosz]

Question	Answer
Tickets	N/A
Bug fix?	yes
New feature?	no
BC breaks?	no
Tests pass?	TBD
Doc needed?	no
License	GPL-2.0

This PR fixes minimum-stability for:

• ezpublish-kernel
• repository-forms
• ezplatform-admin-ui-modules

as the provided versions are beta versions and composer install fails.

Checklist:

• Coding standards ($ composer fix-cs)
• Ready for Code Review

[lserwatka]

@alongosz Why do you need this? We never had to update composer explicitly for beta. I'm against this change.

[andrerom]

@alongosz Do you need this for tests? If so can for instance be overridden in require. As for full install like @lserwatka mentions we only set this in root package and do not need to set it elsewhere.

[alongosz]

@lserwatka @andrerom not just for tests. In general composer install fails on that repository, which results in CI failure for every PR against master.

I'm not sure why this worked for you before. It shouldn't. Technically packages like ezpublish-kernel in version 7.1 or higher (^7.1) don't exist yet. They're in beta. This could be fixed also by specifying minimum-stability here, but I think doing that explicitly for every package that is in beta is better.

Of course after stable release we could go back to non-beta requirement.

[lserwatka]

@alongosz you should never run composer install on this repo. Please always use meta for this.

[andrerom]

you should never run composer install on this repo

@lserwatka he/we has to for unit tests, so he has a point here. Requirements where raised here which broke CI (Travis, local, Jenkins, ..), as 7.1 does not exits in stable packages yet.

[alongosz]

@lserwatka
What can I say... For me concept of having a package with composer.json but being unable to run composer install is just wrong. Sorry to disagree.

Another reason:
The usual development process involves loading ezplatform-admin-ui as a separate PhpStorm project. For type hinting to work I need to install dependencies. I can't imagine loading entire meta every time I need to work on a dependency. Even PhpStorm complains when trying to modify any code in a dependency.

And once again I need to emphasise what @andrerom also just mentioned: Continuous Integration fails right now. See other PRs: #386, #387, #388, #390, #391.

[lserwatka]

@alongosz @andrerom we have zillion repositories to maintain, and updating every repo for composer.json is extremely counter productive for the entire engineering team. We need to find a better way to run tests. I can't imagine updating all packages in every repo for beta/rc release. This is not how we maximise resources. Please figure out how to run the tests without involving changes to composer.json

[lserwatka]

And one more thing. We worked on this repo for 3 months without any issue or need to change to composer.json, so we should figure out why suddenly this stopped working.

[andrerom]

And one more thing. We worked on this repo for 3 months without any issue or need to change to composer.json, so we should figure out why suddenly this stopped working.

It's because the CI/tests break happened 2 days ago, it was fine until then: ed0f12b

[andrerom]

So not saying we need to accept the proposed patch here, and I don't think @alongosz does either, he just tries to solve the failures.

Maybe rather skip bumping requirements unless they are real feature requirements. And in those cases do it as a PR (the feature PR introducing the bumped dependency ideally, otherwise separate PR), and as usual only merge once it's green.

[andrerom]

This could also be @dev if this package really depend on 7.1 now, same for changes below expect maybe ezsystems/ezplatform-admin-ui-modules which might need to be built like asset repos (?).

@dev is what we would have used if this was done as part of PR that introduced 7.1 dependency.

[lserwatka]

I think we have one more problem here. If bumping requirements broke the installation process, then we were unit testing on wrong kernel. What was even more wrong. For time being we need to add @dev suffix then until we figure out better way.

[alongosz]

So not saying we need to accept the proposed patch here, and I don't think @alongosz does either, he just tries to solve the failures.

I'll try to force stability in travis.yml. That might work. Need to check. Still disagree on the whole concept though :)

Maybe rather skip bumping requirements unless they are real feature requirements. And in those cases do it as a PR (the feature PR introducing the bumped dependency ideally, otherwise separate PR), and as usual only merge once it's green.

In this case they might be really needed (Custom Tags 🙃 ).
I just think we tend to ignore CI failures. They fail for a while, then they don't (once we release they will not fail).

If bumping requirements broke the installation process, then we were unit testing on wrong kernel.

That's right. That could be also my point :-)

[lserwatka]

@alongosz @andrerom pushed fix to master with @dev but again, we need to optimize this as we can't release this package with @dev so again we need to restore it.

[andrerom]

uf bumping requirements broke the installation process, then we were unit testing on wrong kernel. What was even more wrong

depends, where there actual dependencies on 7.1 kernel? If package only requries 7.0 and higher, then it was not completely wrong at-least (but see below, kernel should probably be dev)

@alongosz @andrerom pushed fix to master with @dev but again, we need to optimize this as we can't release this package with @dev so again we need to restore it.

👍 Yeah we typically remove the @dev part after some package is stable. But on kernel requirement we often keep it to always test against incomming release to catch issues early:

• https://github.com/ezsystems/ezplatform-http-cache/blob/master/composer.json#L13
• https://github.com/ezsystems/ezplatform-solr-search-engine/blob/master/composer.json#L15
• ...