Fix PHP 8.2 and Other Related Issues #640
Conversation
|
I think you should extract into another PR the actual changes of the code and keep this one for the tooling |
mralaminahamed
changed the title
This reverts commit f4e69f4.
mralaminahamed
changed the title
mralaminahamed
changed the title
Co-authored-by: William Desportes <[email protected]>
|
What's the expected timeframe for merging this PR into the master branch? |
|
Hi @mralaminahamed , Sorry for the late response. We will check your PR, thank you for the contribution! |
… different types
|
@williamdes, I've made the changes based on your feedback. Could you please take another look and let me know if there's anything else that needs to be addressed? Thanks for your time and guidance! |
|
Hi @iulian03, I hope you are doing well. I wanted to follow up on the status of this pull request (#640) that addresses the PHP 8.2 compatibility issues and other related problems. It's been over a month since I submitted the pull request, and I've been actively working on incorporating the valuable feedback provided by @williamdes and making the necessary changes. The latest updates have been reviewed and approved by @williamdes, indicating that the pull request is ready for final review and merging. I understand that you and the team might have a busy schedule, but I kindly request an update on the timeline for reviewing and merging these changes. As a developer relying on this SDK, it's crucial to have it compatible with the latest PHP versions, and I believe many others in the community would also benefit from these fixes. Additionally, I wanted to inquire about your plans for the next release of the Mangopay PHP SDK. It would be great if you could share any information on when we can expect a new version that includes these compatibility enhancements. |
|
It'd be great if you could add tests against php 8.3 as well (tho it seems to work fine from my experience, without the MR) |
|
Thanks for your feedback, and I will try to add test cases for php 8.3. |
# Conflicts: # MangoPay/Libraries/ApiBase.php
| * @see \MangoPay\KycDocumentStatus | ||
| * @phpstan-var \MangoPay\KycDocumentStatus::* | ||
| */ | ||
| public $TypeLabel; |
There was a problem hiding this comment.
Why did you add this property? I don't see it in the docs :D
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * | ||
| * @var string | ||
| */ | ||
| public $StatusLabel; |
There was a problem hiding this comment.
Why did you add this property? I don't see it in the docs :D (correct me if I'm wrong)
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * | ||
| * @var string | ||
| */ | ||
| public $SecureMode; |
There was a problem hiding this comment.
What is the reason for adding this to the PayIn class?
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * @see https://mangopay.com/docs/endpoints/direct-card-payins#direct-card-payin-object | ||
| * @var string | ||
| */ | ||
| public $Requested3DSVersion; |
There was a problem hiding this comment.
What is the reason for adding this to the PayIn class?
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * The IP address of the end user initiating the transaction, in IPV4 or IPV6 format. | ||
| * @var string | ||
| */ | ||
| public $IpAddress; |
There was a problem hiding this comment.
What is the reason for adding this to the PayIn class?
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * One of PayOutPaymentDetails implementations, depending on $PaymentType | ||
| * @var object | ||
| */ | ||
| public $PayoutPaymentDetails; |
There was a problem hiding this comment.
What is the reason for adding this to the PayOut class? We already have it below.
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * 'STANDARD', 'INSTANT_PAYMENT', 'INSTANT_PAYMENT_ONLY' | ||
| * @var string | ||
| */ | ||
| public $PayoutModeRequested; |
There was a problem hiding this comment.
What is the reason for adding this to the PayOut class?
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * Message related to the refund | ||
| * @var string | ||
| */ | ||
| public $RefundReasonMessage; |
There was a problem hiding this comment.
Why did you add this param? I don't see it in the docs (correct me if I'm wrong)
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| * The output status of Declaration status. | ||
| * @var string | ||
| */ | ||
| public $OutputStatus; |
There was a problem hiding this comment.
Why did you add this param? I don't see it in the docs (correct me if I'm wrong)
There was a problem hiding this comment.
This property was identified in both the documentation and PHP SDK. I verified its existence through debug logs and implemented it accordingly.
| RUN docker-php-source delete | ||
|
|
||
| # Install composer | ||
| COPY --from=composer:2 /usr/bin/composer /usr/bin/composer |
There was a problem hiding this comment.
Why is this step different from the other versions?
There was a problem hiding this comment.
Would you please check the screenshot?
There was a problem hiding this comment.
@iulian03 this is the recommended version by composer
|
@iulian03 , if there are no issues, may I close this PR? I originally created it to address PHP 8.2 compatibility issues in our production environment with our product. However, it has been pending review for some time, and there have been significant changes to the documentation since then. Some of the proposed changes may no longer align with the current documentation structure. |
Changes Proposed in this Pull Request:
Related Issue(s) and PR(s):
How to Test the Changes in this Pull Request:
Changelog Entry:
Self-Review Checklist:
Additional Notes: