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.

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

Forward port #2810 #2885

Closed
wants to merge 2 commits into from
Closed

Forward port #2810 #2885

wants to merge 2 commits into from

Conversation

fballiano
Copy link
Contributor

#2810 was left out of #2766 because of conflicts, here's the merge in its own PR, please check :-)

@github-actions github-actions bot added the Component: Sales Relates to Mage_Sales label Jan 3, 2023
@@ -441,42 +441,42 @@ class Mage_Sales_Model_Order extends Mage_Sales_Model_Abstract
protected $_eventObject = 'order';

/**
* @var Mage_Sales_Model_Resource_Order_Address_Collection|Mage_Sales_Model_Order_Address[]|null
Copy link
Contributor

Choose a reason for hiding this comment

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

keep as is in v20

}
}
return $items;
}

/**
* @param int $itemId
* @return Mage_Sales_Model_Order_Item|null
Copy link
Contributor

Choose a reason for hiding this comment

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

keep as is in v20 (and maybe backport this change to v19?)

@@ -115,14 +111,12 @@ public function _construct()
* Init mapping array of short fields to
* its full names
*
* @return $this
Copy link
Contributor

Choose a reason for hiding this comment

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

keep as is in v20, and backport to v19

@tmotyl
Copy link
Contributor

tmotyl commented Jan 3, 2023

added comments, most of the changes should not be applied on v20.

@fballiano
Copy link
Contributor Author

@sreichel reading @tmotyl comments, IMHO we should revert #2810, it was done for an easier sync and it seems it's making it more problematic.

@tmotyl
Copy link
Contributor

tmotyl commented Jan 3, 2023

@fballiano I don't see a point in reverting #2810 .
This PR mixes changes which were made in v20 branch on purpose (e.g. OldFieldsMap) with ones that makes sense to be ported to v20. And some (typehints) which make sense to port from v20 to v19.

@fballiano
Copy link
Contributor Author

@tmotyl I think I fixed all your comments. At this point I don't understand what this PR was about.

@fballiano
Copy link
Contributor Author

i'm closing this one since i don't know wth i should do

@fballiano fballiano closed this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Sales Relates to Mage_Sales
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants