Bring Parable to 1.0.0 #27
Conversation
… even if superfluous. Add some methods. Fix tests. Remove deprecated stuff.
…nder|from.email|from.name].
…d not assert anything'.
…HANGELOG.md as well.
|
Yo, @caelaris, @jerry1970, @dmvdbrugge, @RickWong and @FaaPz! Tagging y'all since I know you guys well enough to directly ask for feedback. So here goes. I'd add some of you as reviewers, but that includes providing repo Collaborator access and that's a little far for my tastes 😉 Feedback on these changes would be much appreciated! I'll be working on and off the next month on the check list, but if there's anything you feel strongly about so far, let me know 🙂 Note: this includes meta-things outside of code changes, general things about releasing a 1.0.0 that you would prefer to see go along with it. |
|
😎🎉⚡️ nice |
|
Congrats! 🎉 |
|
It's not a release yet, @jerry1970 😉 I created this PR mostly because I'd like to finally do all those things I haven't had to worry about while pre-releasing this thing for years 😂 |
|
Looking for:
Etcetera. Anything you can think of, this is the moment to let it all out! Man, this can't go wrong 😅 |
src/Event/Hook.php
Outdated
| @@ -8,7 +8,7 @@ class Hook | |||
| protected $hooks = []; | |||
|
|
|||
| /** | |||
| * Add hook referencing $closure to $event, returns false if $callable isn't a function | |||
| * Add hook referencing $closure to $event, returns false if $callable isn't a function. | |||
There was a problem hiding this comment.
I think the second half of this comment needs a rewrite, because in no way it returns false.
There was a problem hiding this comment.
Agreed. Good find, will go through comments and try to catch all these mismatches 👍
| const HOOK_INIT_DATABASE_AFTER = "parable_init_database_after"; | ||
| const PARABLE_VERSION = '1.0.0'; | ||
|
|
||
| const HOOK_HTTP_404 = "parable_http_404"; |
There was a problem hiding this comment.
Shouldn't these be either alphabetical or in the exact order they're called? (Assuming whatever document generator you'll be using doesn't do that, if it does then forget it.)
There was a problem hiding this comment.
Honestly I'm pretty sure there is logic behind this madness, but I can't come up with it. That means I'll make this look prettier ;)
There was a problem hiding this comment.
I've re-ordered them mostly alphabetically, but I've kept the before/during/after order in there was well.
src/Console/Output.php
Outdated
|
|
||
| echo str_repeat(PHP_EOL, $count); | ||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Move the cursor forward by $characters places and reset the lineLength. |
There was a problem hiding this comment.
You can remove the and reset the lineLength here and all the other places, now that lineLength is gone.
Should be replaced with and disables line clearing in the places (not this one) where it does.
|
Merged in @dmvdbrugge's PR #29 for dynamic return types plugin for phpstorm. |
…refs that are no longer needed.
- Check if posix_isatty function exists before calling - Get terminal width and height for Windows (both cmd and shell)
Also constants for defaults.
Console fixes for Windows
See new test, dp-testcases with index 5, 6, 7, and 9 fail without these changes.
Fix getOption() and getProvidedValue() for they break on falsy values.
…or calling non-static actions.
…pp rather than arrays.
Break the routes and then unbreak them again.
Things to do:
README.mdto read like a 1.0.0 release and including examples of current use.CONTRIBUTING.mdfile to explain how to contribute and that not every suggestion will make the cut.CHANGELOG.mdto 1.0.0 final.Optional: