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

Cleanup; add toJson and fromJson to ApiContext [#39] #40

Merged
merged 4 commits into from
Aug 22, 2017
Merged

Cleanup; add toJson and fromJson to ApiContext [#39] #40

merged 4 commits into from
Aug 22, 2017

Conversation

dnl-blkv
Copy link
Contributor

No description provided.

@dnl-blkv dnl-blkv added this to the v0.10.0 milestone Aug 22, 2017
OGKevin
OGKevin suggested changes Aug 22, 2017
View reviewed changes
Copy link
Contributor

@OGKevin OGKevin left a comment •
edited

Choose a reason for hiding this comment

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

Fatal on tests php 7.1

@OGKevin
Copy link
Contributor

OGKevin commented Aug 22, 2017

When running tests the following fatal is thrown:

PHP Fatal error:  Access level to bunq\tests\Context\ApiContextTest::$apiContext must be protected (as in class bunq\test\BunqSdkTestBase) or weaker in /Users/khellemun/GitHub/bunq/sdk_php/tests/Context/ApiContextTest.php on line 11
PHP Stack trace:
PHP   1. {main}() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit_TextUI_Command::main() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/phpunit:52
PHP   3. PHPUnit_TextUI_Command->run() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/TextUI/Command.php:116
PHP   4. PHPUnit_TextUI_TestRunner->getTest() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/TextUI/Command.php:138
PHP   5. PHPUnit_Framework_TestSuite->addTestFiles() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/Runner/BaseTestRunner.php:57
PHP   6. PHPUnit_Framework_TestSuite->addTestFile() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/Framework/TestSuite.php:382
PHP   7. PHPUnit_Util_Fileloader::checkAndLoad() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/Framework/TestSuite.php:310
PHP   8. PHPUnit_Util_Fileloader::load() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/Util/Fileloader.php:36
PHP   9. include_once() /Users/khellemun/GitHub/bunq/sdk_php/vendor/phpunit/phpunit/src/Util/Fileloader.php:52

@dnl-blkv
Copy link
Contributor Author

@OGKevin Oops, forgot about that declaration. Fixed.

OGKevin
OGKevin previously approved these changes Aug 22, 2017
View reviewed changes
rubengommers
rubengommers reviewed Aug 22, 2017
View reviewed changes
src/Context/ApiContext.php Outdated
@@ -290,14 +308,23 @@ public function getSessionToken()
/**
* @param string $fileName
*
* @throws BunqException
* @throws BunqException When could not save the context to the given location.
Copy link

Choose a reason for hiding this comment

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

Doesn't "When the context couldn't be saved to the given location" make more sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rubengommers Thanks, fixed!

andrederoos
andrederoos requested changes Aug 22, 2017
View reviewed changes
src/Context/ApiContext.php Outdated
* @return string[][]
* @throws BunqException
* @return string
* @throws BunqException If could not find the file to get the string from.
Copy link
Contributor

Choose a reason for hiding this comment

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

not really english

Copy link
Contributor

Choose a reason for hiding this comment

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

When the context couldn't be loaded from the given location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrederoos fixed! 👍

@andrederoos andrederoos merged commit 36617f7 into develop Aug 22, 2017
@OGKevin OGKevin deleted the 39-to-json-from-json branch August 22, 2017 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rubengommers rubengommers rubengommers left review comments

@andrederoos andrederoos andrederoos approved these changes

@OGKevin OGKevin OGKevin left review comments

Assignees

@andrederoos andrederoos

Labels
enhancement
Projects
None yet
Milestone
v0.10.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dnl-blkv @OGKevin @andrederoos @rubengommers