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

Fix 1.9.3.x upstream merges #409

Merged
merged 1 commit into from
Mar 9, 2018
Merged

Fix 1.9.3.x upstream merges #409

merged 1 commit into from
Mar 9, 2018

Conversation

edannenberg
Copy link
Contributor

The upstream merges for some 1.9.3.x releases introduced a few inconsistencies:

The issues were found by extracting and applying the patch set on top of a vanilla Magento 1.9.3.7 commit, finally doing a directory compare with the current 1.9.3.x branch. The branch used for comparison can be found here

#21 is not in this PR in favour of #387

@edannenberg
Fix 1.9.3.x upstream merges
bb29a49 
The upstream merges for some 1.9.3.x releases introduced a few inconsistencies:

* cms_generate_breadcrumbs event is fired twice
* related to the above dd41230 is applied to wrong code block
* few locale .csv have now escaped single quote entries along with the entries where those got removed in OpenMage#46
* OpenMage#21 got reverted at some point
* var/package dir has 5 obsolete xml files
* slightly different configuration in Sales, SalesRule and Tax config.xml
* Mage_SalesRule_Model_Validator is outdated compared with latest from 1.9.3.7
@seansan
Copy link
Contributor

seansan commented Dec 7, 2017 via email

@edannenberg
Copy link
Contributor Author

Maybe, we need to double check some of these reverted/re-applied changes against PHP7 support ... esp the Sales and SalesRule XML config

The Inchoo extension added a cleaner fix for the .xml workaround, not sure about any BC issues with PHP5.6, else this would be a good addition for this repo as it fixes the issue at the root.

In a recent 193.x. install instance we were seeing the 1 cent rounding error again ... that should have been long gone

Not sure this can ever be truly fixed unless an effort is made to finally use the intended precision for product prices in the db. All that deltaRound shenanigans could be avoided..

@seansan
Copy link
Contributor

seansan commented Dec 7, 2017 via email

@seansan
Copy link
Contributor

seansan commented Feb 8, 2018 •
edited
Loading

+1 ignore my rounding issues ... this was solved. And dont want to block other good things in this change

I will test again after merged (to see if my own problem persists)

@edannenberg edannenberg mentioned this pull request Feb 28, 2018
Merged
@colinmollenhour colinmollenhour merged commit d92a24a into OpenMage:1.9.3.x Mar 9, 2018
@edannenberg edannenberg deleted the fix/upstream-merge branch March 13, 2018 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colinmollenhour colinmollenhour colinmollenhour approved these changes

Assignees
No one assigned
Labels
Cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@edannenberg @seansan @colinmollenhour @sreichel