-
Notifications
You must be signed in to change notification settings - Fork 18
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
ISSUE-2 add CLI to allow easier automation #37
Conversation
needed to add code to startup.php to get script name. is this always save and does it return script name? `elseif ($_SERVER['argv'][0] != '') { // php CLI // TODO: check to see if this is safe and always true $SETTINGS[PSNG_SCRIPT] = $_SERVER['argv'][0];`
@eniad I am thinking that some documentation to accompany this PR would be helpful. Naybe we can augment the README wdyt? |
sitepod-cli.php
Outdated
|
||
ob_start (); // Start buffering output | ||
|
||
require_once (dirname ( __FILE__ ) . '/cron.php'); |
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.
Why should a CLI import the whole cron.php?
sitepod-cli.php
Outdated
// TODO: add verbose text, html, and quiet text flags | ||
$text_only = 1; | ||
|
||
ob_start (); // Start buffering output |
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.
I would suggest to avoid comments like this.
The code should explain itself.
ob_start() is basic function, we don't need to explain it.
ob_get_clean() and strip_tags() are also basic functions.
sitepod-cli.php
Outdated
$results = ob_get_clean (); // Put the buffered output into $results and clear the output buffer | ||
|
||
if ($text_only) { | ||
$results = strip_tags ( $results ); // Strip HTML and PHP tags from results |
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.
Strip HTML and PHP tags from results
In my opinion strip_tags() is a basic function, we don't need to explain it in a comment.
BTW: $results don't have any php tags, I think.
* add more tests * composer update, not install
Hi @sigee I went ahead and removed the unnecessary comments. We still import the /cron.php however. |
@sigee ping if you have time to review... |
@lewismc |
The presence of the ISSUE-2 branch was breaking out Packagist.org compliance. Instead of maintaining it as a branch, this opens a PR for review by the team.
@eniad FYI thanks for the PR.