-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change endpoint #5
Conversation
64ddbb5
to
82c7b65
Compare
d05db85
to
ab6b341
Compare
ab6b341
to
d718cf3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could tag or branch the previous version of the library before merging it into the main, in case someone needs it for the last version on the API
public readonly string $street, | ||
public readonly string $streetNo, | ||
public readonly string $zipCode, | ||
public readonly string $floor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this property be nullable? Otherwise, will it default to an empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an empty string by default
$data['data'], | ||
); | ||
|
||
return new self( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I changed it
The last tag/release 1.0.1 is the same as |
composer.json
Outdated
"symfony/property-access": "6.4.*", | ||
"symfony/property-info": "6.4.*", | ||
"symfony/serializer": "6.4.*", | ||
"webmozart/assert": "^1.11" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is not require for this PR don't bump min versions in require
section
This is public library, maybe someone has symfony 6.1 for example
src/Resources/config/services.yaml
Outdated
Answear\FanCourierBundle\Logger\FanCourierLogger: | ||
shared: false | ||
|
||
Answear\FanCourierBundle\Client\RequestTransformerInterface: '@Answear\FanCourierBundle\Client\RequestTransformer' | ||
Answear\FanCourierBundle\Client\RequestTransformerInterface: '@Answear\FanCourierBundle\Client\RequestTransformer' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
public function supportsNormalization($data, $format = null): bool | ||
{ | ||
return is_object($data) && $this->isEmptyClass($data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this normalizer necessary?
Why not `https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Serializer/Normalizer/CustomNormalizer.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked this, and indeed no normalizer is needed for normalizing requests.
No description provided.