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

Small naming update in grid #9452

Merged
merged 3 commits into from Aug 16, 2018

Conversation

Projects
None yet
6 participants
@sarjon
Member

sarjon commented Aug 15, 2018

Questions Answers
Branch? develop
Description? It changes naming from from using rows to records, e.g. Previous: RowCollection, now RecordCollection. Record - item, that can be retrieved from database or any other data source and displayed in grid as single row.
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? No behaviour has changed. But pages with grid should work as they did before.

This change is Reviewable

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 16, 2018

Contributor

hello, looks good to me, good work, but maybe @mickaelandrieu should validate it
just think about fixing the conflicts first, I guess your branch is late with the develop one

Contributor

jolelievre commented Aug 16, 2018

hello, looks good to me, good work, but maybe @mickaelandrieu should validate it
just think about fixing the conflicts first, I guess your branch is late with the develop one

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 16, 2018

Contributor

@jolelievre @sarjon until the release of 1.7.5-beta it's allowed to rename/break everything that is Grid related. Then... it will need to be gracefully deprecated :)

Contributor

mickaelandrieu commented Aug 16, 2018

@jolelievre @sarjon until the release of 1.7.5-beta it's allowed to rename/break everything that is Grid related. Then... it will need to be gracefully deprecated :)

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 16, 2018

Contributor

There was 1 failure:

  1. Tests\Unit\Core\Grid\Presenter\GridPresenterTest::testGridInstanceIsPresentedAsArray
    Failed asserting that an array has the key 'rows'.

Some extra work for you :-)

Contributor

mickaelandrieu commented Aug 16, 2018

There was 1 failure:

  1. Tests\Unit\Core\Grid\Presenter\GridPresenterTest::testGridInstanceIsPresentedAsArray
    Failed asserting that an array has the key 'rows'.

Some extra work for you :-)

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon
Member

sarjon commented Aug 16, 2018

@mickaelandrieu done. :)

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 16, 2018

Contributor

Hello @marionf I've checked grid pages and found an issue on Logs page but it's not related to his pull request.

LGTM.

Thanks @sarjon

Contributor

mickaelandrieu commented Aug 16, 2018

Hello @marionf I've checked grid pages and found an issue on Logs page but it's not related to his pull request.

LGTM.

Thanks @sarjon

@mickaelandrieu mickaelandrieu merged commit d03eb1d into PrestaShop:develop Aug 16, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Aug 23, 2018

@matks matks added the migration label Sep 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment