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

Fix for timings command #2317

Closed
wants to merge 4 commits into from

Conversation

@Frago9876543210
Copy link
Contributor

commented Jul 22, 2018

Introduction

This pull request corrects uploading for timings command.

Follow-up

pocketmine.command.timings.timingsUpload can now be deleted

@lukeeey
Copy link
Contributor

left a comment

Just wondering could the user agent have a slash in the middle like other UAs? E.g. PocketMine-MP/3.6.9

break;
}
if(isset($result[0])){
$pasteId = json_decode($result[0], true)["id"];

This comment has been minimized.

Copy link
@dktapps

dktapps Jul 23, 2018

Member

careful with your error handling here! json_decode() could return false, or the id offset might not exist.

@dktapps dktapps self-assigned this Jul 24, 2018

break;
}
if(isset($result[0])){
$pasteId = json_decode($result[0])->id ?? 0;

This comment has been minimized.

Copy link
@dktapps

dktapps Jul 25, 2018

Member

and if json_decode() returns false, what then?

This comment has been minimized.

Copy link
@Frago9876543210

Frago9876543210 Jul 25, 2018

Author Contributor

@dktapps try this code

$obj = (object) false;
var_dump($obj);
var_dump($obj->id ?? 0);

?? will work as isset and prevent error

This comment has been minimized.

Copy link
@dktapps

dktapps Jul 25, 2018

Member

your error handling should be explicit... and the paste ID should not become zero otherwise a broken link will be reported.

@Frago9876543210

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2018

Like this?

@dktapps

This comment has been minimized.

Copy link
Member

commented Jul 25, 2018

add an isset($response["id"]) pls

@dktapps

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

Merged as a97c7d3 to 3.0, 3.1, 3.2 and master. Not merging directly since the PR branch is written on top of master. Thanks!

@dktapps dktapps closed this Jul 30, 2018

@Frago9876543210 Frago9876543210 deleted the Frago9876543210:patch-1 branch Jul 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.