Fixes json decode two times #5
Conversation
|
Kicking StyleCI |
|
Errr. If the original job is still pending, styleci will just queue the next behind it. I'll check the queue processing now... |
|
You seem to have crashed it... |
|
Oh ok I thought it wasn't being processed. |
|
I'm killing the queue worker... |
|
The log is millions of lines long... |
|
Sorry :( |
|
lol, it wasn't the open close that caused it btw :) |
|
Lol what was? |
|
It seems to be the fact you've used an upper case I in the branch name. |
|
One sec while I investigate... |
|
Nice bug :) |
|
lol, I'll try running |
|
errr, https://i.starbs.net/1ry2, i can't rememer which one is this repo, lol |
|
LOL |
|
It's dd22d4c7a395673687c289eeea3f34882e498f80. :) |
|
There you go :) |
|
Well that fixed it. Very odd. :/ |
|
I think I need to get styleci to just delete the cloned repo and re-clone it if we get a git error. That would probably resolve such issues. |
|
That's weird so it wasn't the upper case? |
|
Yeah that would be cool, like a retry. |
| @@ -140,11 +140,11 @@ protected function parseResponse($body) | |||
| */ | |||
| protected function responseError($rawResponse) | |||
| { | |||
| if (! $this->isJson($rawResponse->getBody())) { | |||
| return $this->jsonError($rawResponse); | |||
| if ($json = $this->isJson($rawResponse->getBody())) { | |||
There was a problem hiding this comment.
Asignment inside if statements is generally regarded as bad. Symfony doesn't allow it for example.
There was a problem hiding this comment.
I didn't know :)
$json = $this->isJson($rawResponse->getBody())
return $json ?: $this->jsonError($rawResponse);Better?
There was a problem hiding this comment.
If you do that, you could actually just do a one liner.
There was a problem hiding this comment.
Done return $this->isJson($rawResponse->getBody()) ?: $this->jsonError($rawResponse);
No, I don't think it was. I have no idea what caused this. |
joecohens
force-pushed
the
fix/Inefficiency
branch
2 times, most recently
from
5bc672d
to
0d94cf9
Compare
| @@ -193,13 +189,17 @@ public function jsonError($rawResponse) | |||
| * | |||
| * @param string $string | |||
| * | |||
| * @return bool | |||
| * @return bool|array | |||
| */ | |||
| protected function isJson($string) | |||
There was a problem hiding this comment.
isJson isn't really the correct method name now
There was a problem hiding this comment.
That's right it's not returning a bool only anymore.. validateJson() or isValidJson?
There was a problem hiding this comment.
getJson() would be better surely?
There was a problem hiding this comment.
and doesn't json_decode return null on error anyway, so this entire method is a bit over kill
| @@ -167,13 +163,17 @@ public function jsonError($rawResponse) | |||
| * | |||
| * @param string $string | |||
| * | |||
| * @return bool | |||
| * @return bool|array | |||
There was a problem hiding this comment.
array|bool would be better :)
joecohens
force-pushed
the
fix/Inefficiency
branch
from
0d94cf9
to
db5c28c
Compare
joecohens
force-pushed
the
fix/Inefficiency
branch
from
db5c28c
to
3650957
Compare
|
Done. Merging this thank you very much sir! :) |
|
:) |
|
I'm working on creating a decorator for the repository class in styleci fixer that will re-entry stuff and delete the repo if it's damaged ect. |
|
Awesome!!!! |
|
I think we might even need to move this stuff to a new package. It's getting quite large now... |
|
Namespaces are properly massive now, lol: https://i.starbs.net/AW67. |
|
LOL really massive and hard to write :) |
|
Hmm. I'm going to drop "Failed" from the exception names. I'm also going to move the "StyleCI\Fixer\Git" namespace to "StyleCI\Git" at a later point too. :) |
|
👍 Yeah I like it better. |
Fixes https://github.com/dinkbit/notifyme/issues/4
//cc @GrahamCampbell