-
-
Notifications
You must be signed in to change notification settings - Fork 222
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
META 9.0 cleanup todos; commented code; commented/skipped tests; check php inline docs; #4478
Comments
For example the code says here "CREATE METHODS FÜR ALLE COMMANDS" should we do this actually? -> solved via #4489 ✅ |
Another good one: neos-development-collection/Neos.Neos/Classes/Domain/Model/Site.php Lines 158 to 159 in 53a0d13
|
@pKallert noted that we match unused methods
|
cleanup todos. There were many todo comments added from 8.3 to 9.0. Some of those are probably already resolved and were just early thoughts but i saw here and there a few that probably need to be addressed. Either way we should reduce the todos - especially if they are outdated.
neos-development-collection/Neos.Neos/Classes/Controller/Frontend/NodeController.php
Line 364 in 5e485ee
cleanup dangerous
assert
expressions. Instead of using php docs var tag/** @var
we started using phpsassert
. But we should make sure that we never assert anything that could possibly really fail, as for this cases we should throw a real exception.cleanup commented code. Some implementations are partially commented out as they havent been adjusted to work with Neos 9.0 yet and to avoid linting errors. We should make sure we know about all places and fix them or remove them.
cleanup commented/skipped tests. We started to write tests in behat. The old Neos.Neos functional tests (also regarding fusion prototypes) should be migrated to behat. Not necessarily 1 to 1 but to cover all important features / usecases.
cleanup unused new code
neos-development-collection/Neos.ContentRepository.Core/Classes/Dimension/ContentDimension.php
Line 99 in 88cb1fd
neos-development-collection/Neos.ContentRepository.Core/Classes/Feature/NodeAggregateCommandHandler.php
Line 186 in a2a0447
neos-development-collection/Neos.ContentRepository.Core/Classes/Feature/Common/ConstraintChecks.php
Line 191 in 7edbbdc
NodeVariationInternals
check php inline docs. References via
{@see fooo}
should link to existent places. I just found out that our main read model - the beloved node - had long unnoticed misleading documentation, because the docs were originally written for methods and not properties. (Maybe leverage phpstan to check@see
annotations?neos-development-collection/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Node.php
Lines 44 to 52 in 71e64ed
cleanup dead doc generation
Neos\\ContentRepository\\
#4862cleanup dead policy yaml regex
cleanup dead imports TASK: Optimise php imports in Neos 9 code #4747
everything around
NodeReferenceSnapshot
is marked wip and todo? So does copying nodes work if they have references?neos-development-collection/Neos.ContentRepository.Core/Classes/Feature/NodeDuplication/Dto/NodeReferenceSnapshot.php
Line 21 in 0fe7b5a
todos that are not marked as todo but just via
?
and a newline\n
likeneos-development-collection/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/VisibilityConstraints.php
Line 20 in 3a7aa79
check for
Todo
string in exception messages and enhance details or create a dedicated exception subclass.unify license headers Code Style: Unify php file licence headers and their position #5138
Change workspace seems untested BUGFIX: Change workspace runs into endless while-loop #5184 (comment)
Adjust readme of CR Core package that its NOT wip anymore and also in eventstore package description
The text was updated successfully, but these errors were encountered: