search

splitio / php-client

PHP SDK client for Split Software
https://split.io
Other
16 stars 12 forks source link

[BUG] - Update ClientInterface::getTreatmentWithConfig phpdoc #192

Closed Ferror closed 2 years ago

Ferror commented 2 years ago

The phpdocs of ClientInterface are invalid. They state opposite things. Therefore cannot be treated as valid. Please review the SDK for more of the issues.

Ferror commented 2 years ago

@mmelograno Can you get a quick look at it :)

mmelograno commented 2 years ago

Hi @ferror, thanks for raising this.

We will update the phpdoc replacing string for array.

Could you please be more specific under what circumstances the method getTreatmentWithConfig returns null?

Thanks,

Ferror commented 2 years ago

Maybe it does not return null. IDK. The problem is inconsistency.

https://github.com/splitio/php-client/pull/192/files#diff-e58cce9d559032c1d7b464d26acbbbadb14cf6aae6da10362a91612fe7757fdaL70-L73

It says it does not return null, and then two lines lower. "It will return null"

mmelograno commented 2 years ago

GetTreatmentsWithConfig returns an object which contains the treatment result and config. It says that inner object config can be null. The return object (the parent one) is never null. There is more explanations and samples in our official documentation

Ferror commented 2 years ago

Okey. That makes more sense. But we should improve readability - that will do.

Btw it's not returning an object. It's an array. So the phpdoc specifying "string" is still invalid I guess.

But creating a DTO for that would be a BC break. Would you be open to adding something like that to the next version?

mmelograno commented 2 years ago

php 7.1.5 has been released with the updates on ClientInterface.

Cheers,