IBX-10507: Updated method signatures to use nullable type hints for improved code readability and consistency. #636
Conversation
This file contains hidden or 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
… readability and consistency.
wiewiurdp
changed the title
… readability and consistency.
alongosz
reviewed
Aug 18, 2025
| * @param string|null $field | ||
| * @param mixed $value | ||
| */ | ||
| private function createAndPublishContent($field, $value) |
There was a problem hiding this comment.
can we use strict types here instead?
| $imageFileList: '@Ibexa\Bundle\Core\Imagine\VariationPurger\LegacyStorageImageFileList' | ||
| $variationPathGenerator: '@ibexa.image_alias.variation_path_generator' | ||
| $filterConfiguration: "@liip_imagine.filter.configuration" | ||
| $imagesDir: "%ibexa.site_access.config.default.image.published_images_dir%" |
There was a problem hiding this comment.
While at it, for consistency:
Suggested change
| $imagesDir: "%ibexa.site_access.config.default.image.published_images_dir%" | |
| $imagesDir: '%ibexa.site_access.config.default.image.published_images_dir%' |
| if ($path[0] === '/' && $this->siteAccess->matcher instanceof SiteAccess\URILexer) { | ||
| $path = $this->siteAccess->matcher->analyseLink($path); | ||
| if ( | ||
| $this->siteAccess === null |
There was a problem hiding this comment.
I'd actually prefer here to see:
Suggested change
| $this->siteAccess === null | |
| !isset($this->siteAccess) |
because on main the property has a strict type, so if it's not set, trying to do $this->siteAccess === null will result in a fatal error if property was not initialized.
Comment on lines
1079
to
1081
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo|null $versionInfo | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\User\User|null $creator if set given user is used to create the draft - otherwise the current-user is used | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\Language|null if not set the draft is created with the initialLanguage code of the source version or if not present with the main language. | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\Language|null $language if not set the draft is created with the initialLanguage code of the source version or if not present with the main language. |
There was a problem hiding this comment.
If these doc blocks are 1:1 copy of the interface doc for the method, they can be removed here as they're inherited by default.
|
|
||
| /** | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\URLAlias | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\URLAlias $createdUrlAlias |
There was a problem hiding this comment.
These can be simply removed, there's already a strict type there, so this doc block doesn't add any extra value
|
|
||
| /** | ||
| * @param \Ibexa\Contracts\Core\Repository\Values\Content\URLAlias | ||
| * @param array<mixed> $testData |
There was a problem hiding this comment.
Most likely array{\Ibexa\Contracts\Core\Repository\Values\Content\URLAlias, int} (at least based on the variable names later on)
added 4 commits
August 19, 2025 11:39
adamwojs
approved these changes
Aug 19, 2025
barw4
approved these changes
Aug 20, 2025
| * Return a list of user's browser preferred locales directly from Accept-Language header. | ||
| * | ||
| * @param \Symfony\Component\HttpFoundation\Request request to retrieve information from, use current if null | ||
| * @param \Symfony\Component\HttpFoundation\Request $request |
There was a problem hiding this comment.
Obsolete as we have strict typing in method arguments
| * @param int $languageArgumentIndex From 0 and up, so the array index on $arguments. | ||
| */ | ||
| final public function testForLanguagesLookup($method, array $arguments, $return, $languageArgumentIndex, callable $callback = null, int $alwaysAvailableArgumentIndex = null) | ||
| final public function testForLanguagesLookup($method, array $arguments, $return, $languageArgumentIndex, ?callable $callback = null, ?int $alwaysAvailableArgumentIndex = null) |
There was a problem hiding this comment.
Maybe we can split it into multiple lines for readability
| * @param int $languageArgumentIndex From 0 and up, so the array index on $arguments. | ||
| */ | ||
| final public function testForLanguagesPassTrough($method, array $arguments, $return, $languageArgumentIndex, callable $callback = null, int $alwaysAvailableArgumentIndex = null) | ||
| final public function testForLanguagesPassTrough($method, array $arguments, $return, $languageArgumentIndex, ?callable $callback = null, ?int $alwaysAvailableArgumentIndex = null) |
alongosz
approved these changes
Aug 20, 2025
… proper formatting and annotation adjustments
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Description:
Updated method signatures to use nullable type hints for improved code readability and consistency.