Skip to content
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.

Sign up for GitHub

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

Jump to bottom

EZP-31118: Introduced strict types for ContentService #2866

Merged
merged 3 commits into from
Mar 16, 2020
Merged

EZP-31118: Introduced strict types for ContentService #2866

merged 3 commits into from
Mar 16, 2020

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Nov 9, 2019 •
edited
Loading

Question Answer
JIRA issue EZP-31118
Bug/Improvement yes
New feature no
Target version master
BC breaks yes
Tests pass yes
Doc needed yes

This PR introduces strict types for the \eZ\Publish\API\Repository\ContentService

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Fix type in commit message before merge
  • Ask for Code Review.

@adamwojs adamwojs self-assigned this Nov 9, 2019
@adamwojs adamwojs changed the title EZP-31118: Introduced strict types for ContenteService EZP-31118: Introduced strict types for ContentService Nov 9, 2019
mikadamczyk
mikadamczyk approved these changes Nov 12, 2019
View reviewed changes
Copy link
Contributor

@mikadamczyk mikadamczyk left a comment

Choose a reason for hiding this comment

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

docblocks in eZ/Publish/API/Repository/ContentService.php should be improved.

eZ/Publish/API/Repository/ContentService.php Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Outdated Show resolved Hide resolved
eZ/Publish/API/Repository/ContentService.php Outdated Show resolved Hide resolved
ViniTou
ViniTou approved these changes Nov 12, 2019
View reviewed changes
Copy link
Contributor

@ViniTou ViniTou left a comment

Choose a reason for hiding this comment

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

Aside from mentioned phpdocs inconsistencies.

@adamwojs
Copy link
Member Author

PR updated according to @mikadamczyk code review suggestions.

@adamwojs
Copy link
Member Author

PR updated according to @alongosz code review suggestions.

@katarzynazawada katarzynazawada self-assigned this Nov 29, 2019
katarzynazawada
katarzynazawada requested changes Nov 29, 2019
View reviewed changes
Copy link

@katarzynazawada katarzynazawada left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Error when editing/hiding content
Argument 1 passed to eZ\Publish\SPI\Repository\Decorator\ContentServiceDecorator::loadContentInfo() must be of the type int, string given, called in /home/vagrant/master/vendor/ezsystems/ezplatform-admin-ui/src/lib/Form/DataTransformer/ContentInfoTransformer.php on line 78

Adding translation:
Argument 1 passed to eZ\Publish\SPI\Repository\Decorator\ContentServiceDecorator::loadContent() must be of the type int, string given, called in /home/vagrant/master/vendor/ezsystems/ezplatform-admin-ui/src/lib/View/Filter/ContentTranslateViewFilter.php on line 100

This was referenced Nov 29, 2019
Merged
Merged
@adamwojs adamwojs mentioned this pull request Mar 4, 2020
Merged
@alongosz alongosz mentioned this pull request Mar 13, 2020
Merged
7 tasks
@adamwojs adamwojs merged commit 502e0cc into master Mar 16, 2020
@adamwojs adamwojs deleted the content_service_typehint branch March 16, 2020 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@katarzynazawada katarzynazawada katarzynazawada approved these changes

@ViniTou ViniTou ViniTou approved these changes

@alongosz alongosz alongosz approved these changes

@mikadamczyk mikadamczyk mikadamczyk approved these changes

Assignees

@adamwojs adamwojs

@katarzynazawada katarzynazawada

Labels
DX Improvement QA approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adamwojs @mikadamczyk @alongosz @ViniTou @katarzynazawada