Skip to content

Conversation

@mficzel
Copy link
Member

@mficzel mficzel commented Aug 16, 2021

This fixes the following error on master:

Argument 1 passed to Neos\Flow\Annotations\IgnoreValidation::__construct() must be of the type string, null given, called in /var/www/html/Packages/Libraries/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php on line 971
Type: TypeError
  File:
Packages/Framework/Neos.Flow/Classes/Annotations/IgnoreValidation.php
  Line: 41

  Type: Neos\Flow\Core\Booting\Exception\SubProcessException
  Code: 1355480641
  File: Packages/Framework/Neos.Flow/Classes/Core/Booting/Scripts.php
  Line: 712

Since the variable with the annotation is populated in the constructor with a fresh instance the need of this annotation is at least questionable.

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Uhm, why was this ever there?

@kdambekalns
Copy link
Member

Interesting test failures, though. Why is it installing 7.1.x of the Flow dev collection?

@kdambekalns kdambekalns requested review from albe and kitsunet August 16, 2021 15:52
@kdambekalns
Copy link
Member

Regarding the annotation itself: It was added with e09d551 – whether by myself or through a merge hiccup I cannot really tell. But it looks completely disconnected from the actual intention of that change, so I assume it was an oversight.

@kdambekalns
Copy link
Member

Interesting test failures, though. Why is it installing 7.1.x of the Flow dev collection?

Probably fixed with #3412

@mficzel
Copy link
Member Author

mficzel commented Aug 16, 2021

Yes once ##3412 is applied the error here occurs in the tests.

@mficzel mficzel merged commit d1a0f32 into neos:master Aug 17, 2021
@bwaidelich
Copy link
Member

I'm afraid this introduced a regression:

ValidatorResolver::buildBaseValidatorConjunction() explicitly checks for this annotation to skip properties from being validated (see https://github.com/neos/flow-development-collection/blob/a3acde9d245a19d6ac32606ac4edb18bd68fb9e8/Neos.Flow/Classes/Validation/ValidatorResolver.php#L309)

@mficzel
Copy link
Member Author

mficzel commented Nov 19, 2021

If this has a purpose the we have to revert the change and fix the test problem differently.

@albe
Copy link
Member

albe commented Nov 19, 2021

Should be reverted once neos/flow-development-collection#2632 has landed, since the missing annotation caused the regression in #3515

@bwaidelich
Copy link
Member

=> #3516

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.

4 participants