Skip to content
This repository has been archived by the owner on May 13, 2021. It is now read-only.

Update meilisearch-php to v0.14 #47

Merged
merged 5 commits into from
Sep 23, 2020
Merged

Update meilisearch-php to v0.14 #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b177af3
Updated meilisearch-php to v0.14
bensherred Sep 21, 2020
1c71e65
Added http-factory-guzzle and removed .idea from gitignore
bensherred Sep 22, 2020
02d786f
http client installation instruction
shokme Sep 22, 2020
0a03059
Update README.md
shokme Sep 23, 2020
7cc222e
Update README.md
shokme Sep 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,42 @@
$ composer require meilisearch/meilisearch-laravel-scout
```

### HTTP Client

You could use any [PSR-18](https://www.php-fig.org/psr/psr-18/) compatible client to use with this SDK. No additional configurations are required.<br>
A list of compatible HTTP clients and client adapters can be found at [php-http.org](http://docs.php-http.org/en/latest/clients.html).

If you use **Laravel 8** you can skip this section as laravel pre-install Guzzle 7 by default.

Copy link
Member

Choose a reason for hiding this comment

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

Could we add a line for Guzzle 7:

$ composer require guzzlehttp/guzzle

For users using Laravel < 8 but who want to use guzzle 7

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it ok to you ?

Guzzle 7:
```bash
$ composer require guzzlehttp/guzzle
```
If you already have guzzle installed with a version < 7, don't forget to update the version inside your composer.json
```json
"require": {
"guzzlehttp/guzzle": "^7.0"
}
```

Guzzle 6:

```bash
$ composer require php-http/guzzle6-adapter
```

Symfony Http Client:

```bash
$ composer require symfony/http-client nyholm/psr7
```

Curl:

```bash
$ composer require php-http/curl-client nyholm/psr7
```

### Export configuration

```bash
Expand Down
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"require": {
"php": "^7.2.5",
"laravel/scout": "^8.0",
"meilisearch/meilisearch-php": "^0.13"
"meilisearch/meilisearch-php": "^0.14",
"http-interop/http-factory-guzzle": "^1.0"
Copy link
Member

Choose a reason for hiding this comment

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

Aren't http-interop/http-factory-guzzle installed by default in Laravel 8 as Guzzle 7 is? 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think Laravel comes with http-interop/http-factory-guzzle installed, here's the composer.json. I've had a look through those dependencies as well and doesn't look like any require them.

Copy link
Contributor Author

@bensherred bensherred Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It might be worth removing still though as it's only required for Guzzle 6 and 7. I could add a note in the README about it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No the factory is not installed by default.
This is only here to exploit PSR-17 (the php sdk discover the factory and use it for thing like $this->requestFactory->createRequest(...).

I don't know if we should remove it, a part of me is yeah we should because we don't want to "tie" the package with guzzle but in other hand as laravel pre-install guzzle since the version 7 why should we not simplify user life by pre-installing the factory, it will be good for most of laravel users.

For me both are ok, but I prefer option 2 😛

Copy link
Member

Choose a reason for hiding this comment

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

Let's keeping it (option 2). If someone is struggling with this decision, we will see!

},
"require-dev": {
"orchestra/testbench": "^5.0 || ^6.0",
Expand Down