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

Fix deprecation notices with Symfony 4.2 #273

Merged
merged 2 commits into from Dec 4, 2018

Conversation

alcaeus
Copy link
Contributor

@alcaeus alcaeus commented Dec 2, 2018

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related Issue Fixes #271, Fixes #272.
Need Doc update no

As per the issues, creating a tree builder without a root node is deprecated, as is the ContainerAwareCommand. The latter can be replaced with ContainerAwareInterface, but it's better to just explicitly wire dependencies. I've added a BC layer to SearchImportCommand, deprecating instantiating it without a manager registry. If none is given, the command falls back to fetching it from the container.

Since this PR brings in new deprecations, SemVer requires tagging this as a minor release.

@julienbourdeau please let me know if I should populate the changelog and if there should be an upgrading-4.0 document to track upcoming BC breaks in a future major release.

@alcaeus
Copy link
Contributor Author

alcaeus commented Dec 2, 2018

Looks like the PHP 7.1 and 7.2 builds are broken even without this change. Needs to be investigated separately.

Copy link
Contributor

@nunomaduro nunomaduro left a comment

Choose a reason for hiding this comment

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

Thanks for this Pull Request. Let's iterate together over it, I have submitted feedback that must be addressed before merging.

Meanwhile, we should make sure that the CI is testing this package against all possible dependencies: --prefer-lowest, etc.

.gitignore Outdated Show resolved Hide resolved
src/Command/IndexCommand.php Show resolved Hide resolved
src/DependencyInjection/Configuration.php Show resolved Hide resolved
src/Command/SearchImportCommand.php Show resolved Hide resolved
@nunomaduro nunomaduro merged commit c78cf18 into algolia:master Dec 4, 2018
@nunomaduro
Copy link
Contributor

Thanks for your work in this.

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