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
refactoring manage mails. #287
Conversation
Signed-off-by:Thorsten Eurich <thorsten@eurich.de>
Signed-off-by:Thorsten Eurich <thorsten@eurich.de>
Signed-off-by:Thorsten Eurich <thorsten@eurich.de>
Signed-off-by:Thorsten Eurich <thorsten@eurich.de>
fixed the issues, thank you for the review :) |
list ($_GET['te']) = $smcFunc['db_fetch_row']($request); | ||
$smcFunc['db_free_result']($request); | ||
} | ||
$_GET['te'] = list_getMailQueueSize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the $_GET['te'] is used just here and in _pauseMailQueueClear, wouldn't be better to use a variable or something instead of assign it to a $_GET?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that would be better, than to use $_GET directly. Specially with encapsulation of methods, it suits its purpose better if we don't break that encapsulation by superglobals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, please feel free to! Better encapsulation and separation of concerns makes it for a much nicer code to play around with ;) |
This is a refactoring PR, purpose is to:
Please review carefully, I don't have a proper test system set up in order to test the mail queue functionality.
Note: This is just a sample, If you like it I can pick up and redisign other files too:)