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

Backport: #2315 #2746

Merged
merged 3 commits into from
Nov 24, 2022
Merged

Backport: #2315 #2746

merged 3 commits into from
Nov 24, 2022

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Nov 19, 2022 •
edited
Loading

Related Pull Requests

  1. See Order add comment functionality in REST API #2315

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: Sales Relates to Mage_Sales label Nov 19, 2022
@sreichel sreichel changed the title Packport: 2315 Packport: #2315 Nov 19, 2022
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@sreichel
Copy link
Contributor Author

sreichel commented Nov 19, 2022 •
edited
Loading

@elidrissidev when time for it, maybe you can change the code in a new PR to avoid these phpstan errors?

@elidrissidev
Copy link
Member

Phpstan errors already solved in a following PR to my original one.

@elidrissidev
Copy link
Member

Need to cherry-pick d91170b for phpstan fix.

@sreichel
Copy link
Contributor Author

Ok, lets merge this and then cherry-pick #2316

(btw ... i'd set both varibles to null before try/catch to avoid unnecessary calls ... do that later)

@elidrissidev elidrissidev mentioned this pull request Nov 20, 2022
Merged
4 tasks
@elidrissidev
Copy link
Member

Why not add them all in the same PR? I already opened a PR to fix the error with _getLocation()

@sreichel
Copy link
Contributor Author

@elidrissidev this PR should only backport changes made to v.20.

@fballiano fballiano changed the title Packport: #2315 Backport: #2315 Nov 22, 2022
@sreichel sreichel requested a review from kiatng November 23, 2022 18:35
@sreichel sreichel merged commit bab8874 into 1.9.4.x Nov 24, 2022
@sreichel sreichel deleted the packport-2315 branch November 24, 2022 07:03
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit bab8874. ± Comparison against base commit 2afbba9.

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

@elidrissidev elidrissidev elidrissidev left review comments

@fballiano fballiano fballiano approved these changes

@kiatng kiatng kiatng approved these changes

@ADDISON74 ADDISON74 Awaiting requested review from ADDISON74

Assignees
No one assigned
Labels
Component: Sales Relates to Mage_Sales environment phpstan Port: backward
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sreichel @elidrissidev @fballiano @kiatng