slince / shopify-api-php

:rocket: Shopify API Client for PHP
MIT License
128 stars 47 forks source link

Unify returns on model setters #90

Closed jstoone closed 3 years ago

jstoone commented 3 years ago

About

First of all, thank you so much for this package. It's been an absolute blast to work with. Big kudos! :heart:

Only thing I've stumbled upon so far, was a minor inconsistency with a missing return in Variant::setInventoryItemId()-method, not returning $this.

So instead of just fixing that one place, I went ahead and scanned through all Model-DTO's, and fixed up the return types and added any missing returns.

I was planning on writing tests for these, changes but it seems like you're testing the feature using reflection so I took the liberty to skip them. :relaxed::v:

But in either case, please don't hesitate letting me know if you want me to add the tests or if there's any particular section of the package that you would like some help with. :)

Happy monday wishes!

Changes

slince commented 3 years ago

Hello, I think returning $this in the setter of the Model object is outdated; So in the new version I removed all of them; Thank you for your PR; At this time I am preparing to release the official version of 3.x; Hope to get your later PR;