Import Magento Release 1.9.3.0 about magento-lts HOT 15 CLOSED

Iam very sorry, I will not be able to do a proper review before monday.
In case someone else is reviewing this positively, remember to change the default branch.

from magento-lts.

David,

I noticed you committed app\code\core\Mage\Catalog\Model\Resource\Product\Type\Configurable\Attribute\Collection.php.orig

It also appears that PR-72 (commit: ac1e3a7) was merged into this branch. Which caused the conflict, from a quick code review the official code now supports proper sorting so we could revert PR-72 completely.

from magento-lts.

Good on all the other files changed (except for the other .orig files)

from magento-lts.

@alexkirsch thanks for pointing this out. I removed the .orig files, but just to make sure I'm not going crazy, does this file look correct to you (https://github.com/OpenMage/magento-mirror/commits/magento-1.9/app/code/core/Mage/Catalog/Model/Resource/Product/Type/Configurable/Attribute/Collection.php)? It seems Magento incorporated the change from PR-70 (commit: ac1e3a7) in their official release.

from magento-lts.

Merge conflict you found is resolved in a25c721

from magento-lts.

@davidwindell You have the uasort inside the foreach.. which would still work it would simply sort more times than necessary.

I would bring it back in line with upstream

from magento-lts.

Also in the future, I would create an empty branch, and create a PR against the new branch vs doing it as an issue.

from magento-lts.

@alexkirsch Good idea RE: empty branch. We should be in line with upstream on this now.

from magento-lts.

@alexkirsch did you mean me?

from magento-lts.

@davidwindell no, sorry about that.. I should blame the auto complete :)

from magento-lts.

Can anyone else take a look at this so I can move forward with making 1.9.3.0 the default branch? We have a PR for removing the swf files that have been sticking around since we moved to the new process.

from magento-lts.

so, finally did my review.
To actually verify the endresult I took the other route to apply all patches again on the magento-mirror 1.9.3.0 version (which I also verified before)

you can checkout it on https://github.com/Flyingmana/magento-lts/tree/1.9.3.0-based
It uses a mix of cherry-picks, merge "own" and normal merges.

I had two times a merge conflict.

Once for the PHP7 patch, which added most, but did not add ',msrp' to the shipping>after config. (not sure if its important, but probably?)
another one for https://github.com/OpenMage/magento-lts/pull/84/files
here I actually came to a different result https://github.com/Flyingmana/magento-lts/blob/1.9.3.0-based/app/code/core/Mage/Tax/etc/config.xml

                    <tax_subtotal>
                        <class>tax/sales_total_quote_subtotal</class>
                        <after>subtotal,nominal,shipping,freeshipping</after>
                        <before>tax,discount,msrp</before>
                    </tax_subtotal>

Oh, and I also did a diff in general between my and your branch.
the main part:

diff --git a/app/code/core/Mage/Tax/etc/config.xml b/app/code/core/Mage/Tax/etc/config.xml
index 3c6b038..320c6c8 100644
--- a/app/code/core/Mage/Tax/etc/config.xml
+++ b/app/code/core/Mage/Tax/etc/config.xml
@@ -162,7 +162,7 @@
                 <totals>
                     <tax_subtotal>
                         <class>tax/sales_total_quote_subtotal</class>
-                        <after>subtotal</after>
+                        <after>subtotal,nominal,shipping,freeshipping</after>
                         <before>tax,discount,msrp</before>
                     </tax_subtotal>
                     <tax_shipping>
diff --git a/cron.sh b/cron.sh
old mode 100755
new mode 100644
diff --git a/mage b/mage
old mode 100755
new mode 100644
diff --git a/skin/adminhtml/default/default/media/flex.swf b/skin/adminhtml/default/default/media/flex.swf
deleted file mode 100644
index a8ecaa0..0000000
Binary files a/skin/adminhtml/default/default/media/flex.swf and /dev/null differ
diff --git a/skin/adminhtml/default/default/media/uploader.swf b/skin/adminhtml/default/default/media/uploader.swf
deleted file mode 100644
index e38a5a5..0000000
Binary files a/skin/adminhtml/default/default/media/uploader.swf and /dev/null differ
diff --git a/skin/adminhtml/default/default/media/uploaderSingle.swf b/skin/adminhtml/default/default/media/uploaderSingle.swf
deleted file mode 100644
index 3dd31ce..0000000
Binary files a/skin/adminhtml/default/default/media/uploaderSingle.swf and /dev/null differ
diff --git a/skin/frontend/base/default/images/window_overlay.png b/skin/frontend/base/default/images/window_overlay.png
new file mode 100644
index 0000000..681a61d
Binary files /dev/null and b/skin/frontend/base/default/images/window_overlay.png differ
diff --git a/skin/frontend/default/blank/images/window_overlay.png b/skin/frontend/default/blank/images/window_overlay.png
new file mode 100644
index 0000000..681a61d
Binary files /dev/null and b/skin/frontend/default/blank/images/window_overlay.png differ
diff --git a/skin/frontend/default/blue/images/window_overlay.png b/skin/frontend/default/blue/images/window_overlay.png
new file mode 100644
index 0000000..681a61d
Binary files /dev/null and b/skin/frontend/default/blue/images/window_overlay.png differ
diff --git a/skin/frontend/default/default/images/window_overlay.png b/skin/frontend/default/default/images/window_overlay.png
new file mode 100644
index 0000000..681a61d
Binary files /dev/null and b/skin/frontend/default/default/images/window_overlay.png differ
diff --git a/skin/frontend/default/modern/images/window_overlay.png b/skin/frontend/default/modern/images/window_overlay.png
new file mode 100644
index 0000000..681a61d
Binary files /dev/null and b/skin/frontend/default/modern/images/window_overlay.png differ
diff --git a/var/package/Interface_Adminhtml_Default-1.9.1.0.xml b/var/package/Interface_Adminhtml_Default-1.9.1.0.xml
deleted file mode 100644
index d9b067f..0000000

and then a list of package xml files related to "1.9.1.0" which should not exist anymore

Iam not sure about the file modes if I broke something, or if they are wrong in your branch.

But as a whole, I would say we can switch to use your branch as default

from magento-lts.

I did a quick diff myself too, but I did not apply the PHP7 patch.

So basically my spottings with a look at #62:

• window_overlay.pngs were added
• swfs should be removed
• old package xmls should be removed

It's quite astonishing what bugs/typos still exist in official 1.9.3.0....

from magento-lts.

Are we planning on labelling/tagging this release once we fully merge? I was looking at 1 of our test sites today to maybe give it a spin and download full 1.9.3.0 from the repo.

About labelling
Also are we then going to label it as: 1.9.3.0.x???? Where X is an incremental number? We could "package" some fixes/improvements - make it a release like 1.9.3.0.1 for the first one ... after that for every xyz bug fixes we add a release 1.9.3.0.2 1.9.3.0.3 etc ... then when 1.9.4.0 comes out or 1.9.3.1 for that matter we continue as 1.9.3.0.2 1.9.3.0.3 1.9.3.1.4 1.9.3.1.5 (or 1.9.4.0.4 1.9.4.0.5 - this way we know what "version" LTS updates we are on: ie the last number)

from magento-lts.

@infabo The second two points are fixed in PR's #121 #120

@seansan We had discussed that in the past, but decided against it due to how Magento was versioning thier releases (and not coming out with new releases for patches). With the new release pattern Magento is following, that makes more sense - here's a new issue we can use to have a discussion about it: #125

It looks like 1.9.3.0 is approved by a good # of people, so I'll make that the default branch now and we can talk about tagging it in that discussion. Thanks all!

from magento-lts.