Change RequestInterface::getCurlPostFields() return type from ?array to array
#92
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OskarStark
changed the title
RequestInterface::getCurlPostFields() return type from `?arr…RequestInterface::getCurlPostFields() return type from ?array to array
OskarStark
commented
Sep 2, 2024
Comment on lines
-46
to
-50
| #[Depends('testCanBeCrated')] | ||
| public function testGetCurlPostFields(Request $request): void | ||
| { | ||
| $this->assertIsArray($request->getCurlPostFields()); | ||
| } |
There was a problem hiding this comment.
not needed anymore, at is always an array
OskarStark
force-pushed
the
feature/better-return-type
branch
from
0e33d8b to
dd3f3ee
Compare
slunak
reviewed
Sep 3, 2024
src/Client/Request/Request.php
Outdated
| */ | ||
| public function __construct( | ||
| private readonly string $apiUrl, | ||
| private readonly string $method, | ||
| private readonly ?array $curlPostFields = null, | ||
| private readonly ?array $curlPostFields = [], |
There was a problem hiding this comment.
Maybe we can review all instances of $request = new Request($client->buildApiUrl(), Request::GET); and add [] as a third argument?
There was a problem hiding this comment.
Here no need for ?, just array?
Good catch
Maybe we can review all instances of $request = new Request($client->buildApiUrl(), Request::GET); and add [] as a third argument?
We can, but I would not, we need to keep the default value anyway, otherwise it is a BC break
OskarStark
force-pushed
the
feature/better-return-type
branch
from
dd3f3ee to
92216ac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Technically a BC break, but only if you created your own
Requestclass, so low chance