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

Optimise shipping cost computation #8403

Merged
merged 2 commits into from Nov 2, 2017

Conversation

Projects
None yet
7 participants
@jocel1
Contributor

jocel1 commented Oct 9, 2017

Questions Answers
Branch? develop
Description? optimize shipping cost computation by skipping useless queries
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? run tests

Important guidelines


This change is Reviewable

@jocel1 jocel1 requested review from eternoendless and Quetzacoalt91 Oct 9, 2017

@tomlev

This comment has been minimized.

Show comment
Hide comment
@tomlev

tomlev Oct 12, 2017

Member

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

Member

tomlev commented Oct 12, 2017

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@eternoendless eternoendless changed the title from CO: optimise shippingcost computation to Optimise shippingcost computation Oct 19, 2017

@eternoendless eternoendless changed the title from Optimise shippingcost computation to Optimise shipping cost computation Oct 19, 2017

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Oct 19, 2017

Member

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


classes/Cart.php, line 589 at r1 (raw file):

     * @result array Products
     */
    public function getProducts($refresh = false, $id_product = false, $id_country = null, $fullInfos = true)

Missing Phpdoc for this parameter


classes/Cart.php, line 1621 at r1 (raw file):

        foreach ($gifts as $gift) {
            if (
                (int) $gift['id_product_attribute'] === $id_product_attribute &&

When breaking conditions into multiple lines, please put the boolean operator at the start of the line.


Comments from Reviewable

Member

eternoendless commented Oct 19, 2017

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


classes/Cart.php, line 589 at r1 (raw file):

     * @result array Products
     */
    public function getProducts($refresh = false, $id_product = false, $id_country = null, $fullInfos = true)

Missing Phpdoc for this parameter


classes/Cart.php, line 1621 at r1 (raw file):

        foreach ($gifts as $gift) {
            if (
                (int) $gift['id_product_attribute'] === $id_product_attribute &&

When breaking conditions into multiple lines, please put the boolean operator at the start of the line.


Comments from Reviewable

@marionf marionf added QA ✔️ and removed waiting for QA labels Oct 26, 2017

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Oct 31, 2017

Contributor

Hello @jocel1, would you mind to fix last comments from @eternoendless so I can merge before freeze 👍

Contributor

mickaelandrieu commented Oct 31, 2017

Hello @jocel1, would you mind to fix last comments from @eternoendless so I can merge before freeze 👍

@mickaelandrieu mickaelandrieu added this to the 1.7.3.0 milestone Oct 31, 2017

@jocel1

This comment has been minimized.

Show comment
Hide comment
@jocel1

jocel1 Oct 31, 2017

Contributor

done!

Contributor

jocel1 commented Oct 31, 2017

done!

@PrestaShop PrestaShop deleted a comment from codacy-bot Oct 31, 2017

@codacy-bot

This comment has been minimized.

Show comment
Hide comment
@codacy-bot

codacy-bot Oct 31, 2017

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 3
- Added 1
           

Complexity increasing per file
==============================
- classes/Cart.php  1
         

See the complete overview on Codacy

codacy-bot commented Oct 31, 2017

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 3
- Added 1
           

Complexity increasing per file
==============================
- classes/Cart.php  1
         

See the complete overview on Codacy

@Quetzacoalt91

Let's go for merge. Thank you @jocel1

@Quetzacoalt91 Quetzacoalt91 merged commit 8898412 into PrestaShop:develop Nov 2, 2017

2 of 3 checks passed

code-review/reviewable 1 file, 2 discussions left (eternoendless, jocel1, tomlev)
Details
codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment