Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Minor revamp #4

Merged
merged 5 commits into from
Mar 18, 2023
Merged

Minor revamp #4

merged 5 commits into from
Mar 18, 2023

Conversation

CommandString
Copy link
Member

No description provided.

@CommandString CommandString marked this pull request as ready for review March 17, 2023 17:22
@CommandString CommandString mentioned this pull request Mar 17, 2023
{
return json_decode(file_get_contents(__DIR__ . "/../../tools/HttpExceptions/HttpCodes.json"));
return json_decode(file_get_contents(__DIR__ . "/../../tools/HttpExceptions/HttpCodes.json"), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Still using json instead of php to store the data

Copy link
Member Author

Choose a reason for hiding this comment

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

Well this is just an example so I wouldn't worry too much about it 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

youre also using this json in other places, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for generator stuff nothing else

@@ -359,20 +359,20 @@ public static function run(EmitterInterface $emitter = null): void
}

if ($resolved === null) {
if (!self::$emitHttpExceptions) {
if (!is_subclass_of($e, HttpException::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is jank, just catch HttpExceptions seperately

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean just have Catchers?

Copy link
Contributor

Choose a reason for hiding this comment

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

try {
    // ...
} catch (HttpException $e) {
    // ...
} catch (Exception $e) {
    // ...
}

Copy link
Member Author

@CommandString CommandString Mar 18, 2023

Choose a reason for hiding this comment

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

YOU CAN DO THAT!?!? I've been lied to my entire life

@CommandString CommandString merged commit 643a2cb into main Mar 18, 2023
@CommandString CommandString deleted the minor-revamp branch March 18, 2023 11:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants