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

EZP-30898: Dropped dependency on Installers to get InstallType #100

Merged
merged 1 commit into from
Sep 12, 2019

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Aug 28, 2019

Question Answer
JIRA issue EZP-30898
Required by
Bug/Improvement yes
New feature no
Target version master (8.0@dev) for eZ Platform v3.0 Behat tests.
BC breaks no
Tests pass yes
Doc needed no

This PR drops dependency on Installer services defined in the Service Container as there's a simpler method. It's required by ezsystems/ezpublish-kernel#2751 but independent.

I've chosen this solution because I wouldn't recommend maintaining the changed list of service names as there's really no BC promise for that, nor there should be one.

I can improve this one further by moving more responsibility to InstallerType (w/ changed name and dropped abstract), if you want :)

QA

  • Needs testing with all the metas (preferred manually, not all suites, just some random one to check if it works,
  • Needs testing when ran from a dependency, not meta (haven't tried that 😅).

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loos good!

Needs testing with all the metas (preferred manually, not all suites, just some random one to check if it works,

I've done that, works correctly.

Needs testing when ran from a dependency, not meta (haven't tried that 😅).

This PR runs AdminUI tests as a job (https://travis-ci.org/ezsystems/BehatBundle/jobs/577912992), so this is also covered

Thanks 🎉

@alongosz
Copy link
Member Author

Thanks @mnocon! This PR is independent, so feel free to merge it when ready from your perspective (guessing e.g. waiting for another +1).

@alongosz
Copy link
Member Author

Unfortunately with ezsystems/ezplatform-ee-installer#58 getting merged too early, I need to merge this as well to unblock CI. // cc @micszo

@alongosz alongosz merged commit 511699f into master Sep 12, 2019
@alongosz alongosz deleted the ezp-30898-drop-dep-on-installer-services branch September 12, 2019 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants