feat: Add support for to use a object with a union type #580
Conversation
| @@ -61,37 +61,37 @@ class ClientOptions implements ArrayAccess | |||
| { | |||
| use OptionsTrait; | |||
|
|
|||
| private ?string $apiEndpoint; | |||
| public ?string $apiEndpoint; | |||
There was a problem hiding this comment.
This is a highly opinionated change. My believe is that the options objects should be mostly structural and have easy of use for the user and the developers.
If you are in need or wanting to use this object, using an IDE and start typing $options-> should give a quick auto completion that helps the user understand the different options. We should document these values if we decided to continue with this philosophy.
Due this being highly opinionated though, I am willing to return this to private.
There was a problem hiding this comment.
The only concern I have here is that if there's some logic being done in the setters (which is the case for a few of these), it would be lost in setting these manually, which may lead to some confusing behavior.
Hectorhammett
force-pushed
the
client-options
branch
from
efd7432 to
f07cbf5
Compare
There was a problem hiding this comment.
One thing I think we should look into is how to make our reference documentation look better
The first 5 methods in https://googleapis.github.io/gax-php/main/Google/ApiCore/Options/ClientOptions.html are not helpful, the top code example does not really show how to use the class, and the first method that is useful (__construct) is a completely unformatted mess.
I'm not sure what impact it would have making the options public, and if that would help or hurt, but we should look into that also.
|
|
||
| // The constructor of ClientOptions construct creates a `TransportOptions`. | ||
| // We call `toArray` on it and nested items that uses the `optionsTrait` to keep | ||
| // the same behaviour as if it was just an array. |
There was a problem hiding this comment.
We could also make toArray call toArray on those items.... it would be a breaking change, so I think it's a bad idea. It's a shame we didn't do that before! Maybe in a 2.0 version...
| @@ -61,37 +61,37 @@ class ClientOptions implements ArrayAccess | |||
| { | |||
| use OptionsTrait; | |||
|
|
|||
| private ?string $apiEndpoint; | |||
| public ?string $apiEndpoint; | |||
There was a problem hiding this comment.
The only concern I have here is that if there's some logic being done in the setters (which is the case for a few of these), it would be lost in setting these manually, which may lead to some confusing behavior.
|
@Hectorhammett I would suggest, just for safety, that we mark any changing methods as EDIT: Honorable mention here is to also move |
No description provided.