Skip to content

NGSTACK-999 upgrade bundle to ibexa version 5#8

Merged
emodric merged 8 commits intomasterfrom
NGSTACK-999-upgrade-bundle-to-Ibexa-version-5
Oct 23, 2025
Merged

NGSTACK-999 upgrade bundle to ibexa version 5#8
emodric merged 8 commits intomasterfrom
NGSTACK-999-upgrade-bundle-to-Ibexa-version-5

Conversation

@AntePrkacin
Copy link
Contributor

@AntePrkacin AntePrkacin commented Oct 20, 2025

Upgraded bundle to use Ibexa version 5. Also added symfony/form bundle in 'require-dev' section because it is used by Birthday class. For now, I've removed the netgen/ibexa-forms-bundle bundle because it is not yet upgraded to IbexaV5.

In code, I've added a few missing return types for some methods and types for constants. Also used constructor promotion in one class.

Upgraded tests to be compatible with PHP8 and PHPUnit12 - this mainly involved upgrading the @covers annotations to #[Covers] attributes.

@AntePrkacin AntePrkacin requested a review from emodric October 20, 2025 08:12
@AntePrkacin AntePrkacin self-assigned this Oct 20, 2025
* @param DateTimeImmutable|string|null $date Date as a DateTime object or string in Y-m-d format
*/
public function __construct($date = null)
public function __construct(public DateTimeImmutable|string|null $date = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change in behaviour. Previously $date property didn't allow strings and now it does due to constructor promotion.

We should revert back to the old way, since Value::$date still needs to be only DateTimeImmutable|null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How didn't the $date property allow strings before? The PHPDoc contained string as one of the allowed types - that's the reason why I put that $date property can be string.

If you want, I can remove the string type both from the constructor and from the PHPDoc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not talking about the constructor argument, but class property Value::$date, which was previously declared as public ?DateTimeImmutable $date = null;. Constructor promotion makes this property now accept strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, I get it now. I changed the $date property to not be promoted by constructor (like it was before): 3e07eaa

composer.json Outdated
"phpunit/phpunit": "^9.0",
"netgen/ibexa-forms-bundle": "^4.0"
"phpunit/phpunit": "^12.0",
"symfony/form": "^7.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symfony/form should go to require section, since it is not a dev dependency, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I would leave it in require-dev section. The Form/FieldTypeHandler/Birthday class is the only one that uses the symfony/form bundle and the only one that uses netgen/ibexa-forms-bundle. If netgen/ibexa-forms-bundle was in require-dev section, then I would put the symfony/form bundle in the same section as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, I wouldn't add symfony/form at all, since it is required implicitly by ezforms, once we upgrade it. Same goes for the other repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

255bc99 Removed symfony/form bundle from composer.json file

@AntePrkacin
Copy link
Contributor Author

@emodric
Added back the netgen/ibexa-forms-bundle to require-dev section of composer.json file: 57ec941

@emodric emodric merged commit 0d1aac3 into master Oct 23, 2025
2 checks passed
@emodric
Copy link
Member

emodric commented Oct 23, 2025

Thanks @AntePrkacin !

@emodric emodric deleted the NGSTACK-999-upgrade-bundle-to-Ibexa-version-5 branch October 23, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants