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

V2 omnipay mollie #49

Merged
merged 29 commits into from
Jul 24, 2018
Merged

V2 omnipay mollie #49

merged 29 commits into from
Jul 24, 2018

Conversation

ibudasov
Copy link

No description provided.

@Smitsel Smitsel mentioned this pull request Jul 23, 2018
composer.json Outdated
"require": {
"omnipay/common": "^3"
},
"require-dev": {
"omnipay/tests": "^3",
"squizlabs/php_codesniffer": "^3",
"phpro/grumphp": "^0.14"
"phpro/grumphp": "^0.14",
"phpmd/phpmd": "@stable",
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide versions for this? Or is there a specific reason to not require a version?

composer.json Outdated
@@ -29,13 +29,19 @@
"autoload": {
"psr-4": { "Omnipay\\Mollie\\" : "src/" }
},
"autoload-dev": {
"psr-4": { "Omnipay\\Mollie\\Test\\": "tests/" }
},
"require": {
"omnipay/common": "^3"
},
"require-dev": {
"omnipay/tests": "^3",
Copy link
Member

Choose a reason for hiding this comment

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

Can you bump this to ^3.1, that should fix the lowest dependencies. (I've updated the php-mock-client dependency in 3.1)

Copy link
Member

Choose a reason for hiding this comment

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

(referring to omnipay/tests)

composer.json Outdated
"phpro/grumphp": "^0.14",
"phpmd/phpmd": "@stable",
"overtrue/phplint": "@stable",
"jakub-onderka/php-parallel-lint": "@stable"
},
"extra": {
"branch-alias": {
Copy link
Member

Choose a reason for hiding this comment

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

Can you bump the branch-alias to 5.0.x-dev? Because of breaking changes etc.

src/Gateway.php Outdated
@@ -48,82 +69,82 @@ public function setApiKey($value)

/**
* @param array $parameters
* @return \Omnipay\Mollie\Message\FetchIssuersRequest
* @return AbstractRequest|FetchIssuersRequest
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it redundant to add AbstractRequest as the FetchIssuersRequest already extends that? (Same for the others below)

@barryvdh
Copy link
Member

Thanks! Looks great, just a few nitpicks and it's good to merge :)
A bit too bad that Github doesn't understand all moved files so it's a bit hard to see the actual difference in files.

@barryvdh
Copy link
Member

Sorry, I meant omnipay/tests:^3.1, common was fine already with just ^3

composer.json Outdated
"jakub-onderka/php-parallel-lint": "@stable"
"phpmd/phpmd": "^2",
"overtrue/phplint": "^1",
"jakub-onderka/php-parallel-lint": "^0"
Copy link
Member

Choose a reason for hiding this comment

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

@ibudasov
Copy link
Author

@barryvdh could you please have a look at the pipeline?
What can I do better?

@barryvdh
Copy link
Member

Not really sure why he is complaining about not being able to install 3.0.1
You could try enforcing omnipay/common 3.0.2, not sure if that fixes it though.

@ibudasov
Copy link
Author

Yay! It works!

@barryvdh
Copy link
Member

Cool, so this is ready to merge?

@barryvdh barryvdh merged commit dafdbdd into thephpleague:master Jul 24, 2018
@willemstuursma
Copy link

@roelvanduijnhoven FYI

@willemstuursma willemstuursma deleted the v2-omnipay-mollie branch July 24, 2018 07:53
@barryvdh
Copy link
Member

This is tagged as 5.0.0 now

@roelvanduijnhoven
Copy link

@willemstuursma Thanks :). This helps a lot!

@willemstuursma
Copy link

Np @roelvanduijnhoven. Contact me at willem@mollie.com if you need technical support and I'll get you in touch with the right people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants