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

Drop Monolog dependency from ClientBuilder #674

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

mhujer
Copy link
Contributor

@mhujer mhujer commented Nov 7, 2017

I've removed the helper method and updated the docs as noted in #296.
Should I include the change in the BREAKING_CHANGES.md document? (for which version?)

This change is necessary for analyzing src/ with PHPStan. Otherwise it is trying to load \Monolog\Logger\Logger which is not listed as a dependency in composer.json and therefore PHPStan fails in a spectacular way

(This closes #296, and closes #579)

C:19:"Sami\Renderer\Index":1041:{a:3:{i:0;a:10:{s:20:"Elasticsearch\Client";s:40:"6ac4061ad03df8cddeaec31a8bfb556fc7a33e71";s:27:"Elasticsearch\ClientBuilder";s:40:"6b9f0fd58072ecaf8820c64a087b5ab03b2213d3";s:37:"Elasticsearch\Namespaces\CatNamespace";s:40:"a9d466909dc08564e9c1516162c3a6bb91dbecbe";s:41:"Elasticsearch\Namespaces\ClusterNamespace";s:40:"0934f56b5dfa7978ab1907b6c8a04b0a293ae274";s:41:"Elasticsearch\Namespaces\IndicesNamespace";s:40:"66391991c19f28764c86e9604e231b9ce004c82e";s:40:"Elasticsearch\Namespaces\IngestNamespace";s:40:"b52adeb7071f16cba79cdcc3dac3fa6e53ed62bd";s:39:"Elasticsearch\Namespaces\NodesNamespace";s:40:"da4e71f9d953d00600920c26fe585b6884e45f94";s:40:"Elasticsearch\Namespaces\RemoteNamespace";s:40:"a503f6ea82452e1ea0275a044aeb527bc946616d";s:42:"Elasticsearch\Namespaces\SnapshotNamespace";s:40:"e28a1807789b0fcca3fd6b9712ed713650cf7ac2";s:39:"Elasticsearch\Namespaces\TasksNamespace";s:40:"2de86d7ab409a629320725f6444c76d2a9313c72";}i:1;a:1:{i:0;s:6:"master";}i:2;a:2:{i:0;s:13:"Elasticsearch";i:1;s:24:"Elasticsearch\Namespaces";}}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Result of docs rebuilding)

@mhujer mhujer force-pushed the mh-remove-monolog-dependency branch from 140be34 to 737f297 Compare November 19, 2017 18:57
@mhujer
Copy link
Contributor Author

mhujer commented Nov 19, 2017

Rebased to resolve conflicts.

@mhujer mhujer force-pushed the mh-remove-monolog-dependency branch from 737f297 to 923108c Compare November 30, 2017 17:17
@polyfractal
Copy link
Contributor

Welp, I suppose it's time to drop the default logger. Still seems a shame as it's a nice user-friendliness feature... but I've also seen it cause problems since people try to use it before installing Monolog.

@polyfractal
Copy link
Contributor

I think we should add a blurb to the Breaking Changes doc for master, and I think we can only merge this into Master. Ship sailed for 6.x unfortunately (my fault for the delay) since this would outright break code if someone is relying on the method

@mhujer mhujer force-pushed the mh-remove-monolog-dependency branch 2 times, most recently from 8f9d93c to 40b60af Compare November 30, 2017 22:17
@mhujer mhujer force-pushed the mh-remove-monolog-dependency branch from 40b60af to f8ddeca Compare November 30, 2017 22:19
@mhujer
Copy link
Contributor Author

mhujer commented Nov 30, 2017

We definitely can't remove a method in a minor version :-) so this must go to the master.

I've added a note to the BREAKING_CHANGES.md doc.

@@ -1033,27 +1032,6 @@ $response = $client->deleteTemplate($params);



[[Elasticsearch_ClientputTemplate_putTemplate]]
Copy link
Contributor Author

@mhujer mhujer Nov 30, 2017

Choose a reason for hiding this comment

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

I've rebuilt the docs, therefore this unrelated change.

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.

2 participants