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

added description_for_merchant #10

Merged
merged 5 commits into from
Apr 13, 2022

Conversation

AlexKratky
Copy link
Member

@AlexKratky AlexKratky commented Apr 4, 2022

@AlexKratky AlexKratky requested a review from Triplkrypl April 4, 2022 08:36
@AlexKratky AlexKratky self-assigned this Apr 4, 2022
src/Model/SimplePayment.php Outdated Show resolved Hide resolved
src/Model/SimplePayment.php Outdated Show resolved Hide resolved
@Triplkrypl
Copy link
Member

New response parameter is not realeaset yet, change target branche in to v1.x-stage.

@AlexKratky
Copy link
Member Author

but there is not v1.x-stage, only v1.x-spring and this is for summer release https://github.com/ThePay/api-client/branches/all and I am not sure how to create branch to keep checks (one approval etc.)

@Triplkrypl
Copy link
Member

We have checks for all branches started with v, but if we fix break change here, we can release new PHP property without deploying changes on our server. SDK always return null until we kick out new release but merchants developers can implement new interface simultaneously, while we testing new features. This can speed-up integration process.

@Triplkrypl Triplkrypl added the feature New feature label Apr 4, 2022
@AlexKratky AlexKratky changed the base branch from v1.x to v1.x-summer April 6, 2022 08:10
@AlexKratky AlexKratky requested a review from Triplkrypl April 6, 2022 08:10
Copy link
Member

@Triplkrypl Triplkrypl left a comment

Choose a reason for hiding this comment

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

Break change in SimplePayment is not fixed.

@AlexKratky
Copy link
Member Author

Break change in SimplePayment is not fixed.

but if we release it with summer release, there will be that key in response, so no need to check if key exists, otherwise I can add key exist check, but then we can merge it to v1.x

@Triplkrypl
Copy link
Member

Yes we will always send description_for_merchant key in response, but everyone could create SimplePayment object in their project for some creative reason, we can not prevent of call the constructor outside of our library.
So new required input key can break some code, i hope only unit tests where instance can be used as dummy mock.
This break changes not affect main interface for developers, but we can not predict how they use our code, i truly recommend check new array key and have new feature without break change as possible.

@AlexKratky
Copy link
Member Author

Yes we will always send description_for_merchant key in response, but everyone could create SimplePayment object in their project for some creative reason, we can not prevent of call the constructor outside of our library. So new required input key can break some code, i hope only unit tests where instance can be used as dummy mock. This break changes not affect main interface for developers, but we can not predict how they use our code, i truly recommend check new array key and have new feature without break change as possible.

Well now I understand, however it is not documented to use it like that. Every change in input/output of any method could be BC, even after adding check if key is set, someone can have a test, that will check count of key of method toArray() for example

@AlexKratky AlexKratky merged commit f998a07 into v1.x-summer Apr 13, 2022
@AlexKratky AlexKratky deleted the 3981-payment-detail-description-for-merchant branch April 13, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants