Skip to content

Elasticsearch adapter#208

Open
upchuk wants to merge 2 commits into
whiteoctober:masterfrom
upchuk:ElasticsearchAdapter
Open

Elasticsearch adapter#208
upchuk wants to merge 2 commits into
whiteoctober:masterfrom
upchuk:ElasticsearchAdapter

Conversation

@upchuk

@upchuk upchuk commented May 27, 2016

Copy link
Copy Markdown

Hey. Does this look right? An adapter for the official Elasticsearch PHP library

* @param array $params The array of parameters to use for performing the search.
* @param Client $client The Elasticsearch client.
*/
public function __construct(array $params, $client)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing parameter typehint.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Indeed @sstok . Fixed.

@upchuk

upchuk commented Jun 9, 2016

Copy link
Copy Markdown
Author

@sstok all good?

*
* @var array
*/
private $params;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please use 4 spaces for the indentation to match the coding standards of the library

@stof

stof commented Jun 9, 2016

Copy link
Copy Markdown
Contributor

it would also be great to add some tests

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.

3 participants