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 PHP warnings, notices and errors #22

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MetalCar
Copy link

Hi there,

this project is exactly what I need, so I installed it and there come little PHP errors up.
I fixed them on a very very simple way so that other can use it directly withouth fixing errors or fully configurate that project.
I hope that is ok.

@tomershvueli
Copy link
Owner

tomershvueli commented Dec 15, 2020

@MetalCar Thanks for your PR into homepage! This looks good and I like the idea. Let's just fix up one thing before we merge it - can we have the error.php page have a map of error_codes with their text (right now it'll only have config_missing or something similar), and pass along the error code to the page, instead of passing an entire message through parameters? After that I'll merge it in!

@MetalCar
Copy link
Author

This is my solution to make it possible to add more error-codes and show them.

BTW: It's possibly that I will add a second PR to seperate php logic and template, if it's okay.

<?php


final class ErrorCode extends BaseEnum
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of actual Enum's, let's simplify by doing something like this:
define("CONFIG_NOT_FOUND", "The config.json file wasn't found.");
Then we can get rid of the BaseEnum and translation files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants