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
Change to client_id #29
Conversation
Hi! Appreciate the PR, but it seems to be breaking some tests. If you have the time, please take a look at fixing the tests. Otherwise, I'll take care of it when I'm free. |
Sure thing, I think I've fixed it now but the 7.2 compile error is actually more complicated. It's because of this: Fatal error: Declaration of Castle_TestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /home/travis/build/castle/castle-php/test/Castle.php on line 3 phpUnit setUp hint is void, where your test is not. I suspect this has been broken for a while, and I could add it but probably the whole of 7.2 is broken due to the changes in phpUnit? |
I think you better take over. It wont be 5.6 compatible which you are also
testing for so it's probably best you decide where to go :)
Thanks for the help!
…On Wed, Jul 24, 2019 at 6:39 PM Johanna Larsson ***@***.***> wrote:
@dbfx <https://github.com/dbfx> yeah, adding :void to the end of all the
setup definitions fixes it. I can take over the PR and fix it or you can
apply this diff
diff --git a/test/Castle.php b/test/Castle.php
index 062a6e7..82ecd58 100644
--- a/test/Castle.php
+++ b/test/Castle.php
@@ -2,7 +2,7 @@
abstract class Castle_TestCase extends \PHPUnit\Framework\TestCase
{
- public function setUp()
+ public function setUp(): void
{
Castle::setApiKey('secret');
}
diff --git a/test/CastleTest.php b/test/CastleTest.php
index daa667a..e9b131a 100644
--- a/test/CastleTest.php
+++ b/test/CastleTest.php
@@ -8,7 +8,7 @@ public static function setUpBeforeClass() {
Castle::setApiKey('secretkey');
}
- public function setUp()
+ public function setUp(): void
{
$_SESSION = array();
$_COOKIE = array();
diff --git a/test/ErrorsTest.php b/test/ErrorsTest.php
index c2e7bb7..8a50eae 100644
--- a/test/ErrorsTest.php
+++ b/test/ErrorsTest.php
@@ -3,7 +3,7 @@
class CastleErrorTest extends Castle_TestCase
{
- public function setUp()
+ public function setUp(): void
{
$_SERVER['HTTP_USER_AGENT'] = 'TestAgent';
$_SERVER['REMOTE_ADDR'] = '8.8.8.8';
diff --git a/test/RequestContextTest.php b/test/RequestContextTest.php
index a863431..c081d04 100644
--- a/test/RequestContextTest.php
+++ b/test/RequestContextTest.php
@@ -9,7 +9,7 @@ public static function setUpBeforeClass() {
unset($_SERVER['HTTP_X_FORWARDED_FOR']);
}
- public function setUp() {
+ public function setUp(): void {
$_COOKIE = array();
}
diff --git a/test/RequestTest.php b/test/RequestTest.php
index 742482b..d0f5b24 100644
--- a/test/RequestTest.php
+++ b/test/RequestTest.php
@@ -12,7 +12,7 @@ public static function setUpBeforeClass()
Castle::setUseWhitelist(false);
}
- public function setUp()
+ public function setUp(): void
{
$_COOKIE = array();
$_SESSION = array();
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29?email_source=notifications&email_token=AABMQ6XETHFKAP23YM2MVUDQBCAUXA5CNFSM4IGEFLJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2W5HGY#issuecomment-514708379>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABMQ6S23JQGLL6LXMTQIWLQBCAUXANCNFSM4IGEFLJA>
.
|
@dbfx the mystery is how the 7.2 builds ran successfully last year! I'll try to dig into it and get back to you as soon as possible |
@dbfx alright, we've decided to drop support for PHP5 and if you rebase on master you should be all green. We'll make a major release after this. |
Great, I think that's best! Thanks for the help :)
…On Thu, Jul 25, 2019 at 12:00 PM Johanna Larsson ***@***.***> wrote:
@dbfx <https://github.com/dbfx> alright, we've decided to drop support
for PHP5 and if you rebase on master you should be all green. We'll make a
major release after this.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29?email_source=notifications&email_token=AABMQ6S7BRDT5FWZI5QKQKDQBF2TPA5CNFSM4IGEFLJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2Y75PQ#issuecomment-514981566>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABMQ6WW7F2A72H37XC6UX3QBF2TPANCNFSM4IGEFLJA>
.
|
I was trying to be fancy and update your fork branch. No idea what happened but not able to reopen this. Created new PR with your changes #31 |
Castle seems to require snakecase and not camelcase.