Rename CLI options for future expansion #1742
Conversation
There was a problem hiding this comment.
Potentially a "breaking change" for anyone who's got used to using dancer2 gen -a MyName but I can see the logic behind it, and if we're going to make a change like that, a major milestone version is the time :)
I do wonder if we should continue to support the old form at least for now as an alternative, though? (Not sure how easily/possible that is via CLI::Osprey, not very familiar with it)
It'll want to be made clear in the changelog that dancer2 gen -a MyApp now needs to be dancer2 gen -n MyApp
If we're making a confusing/breaking change here, should we add an arg for what we're generating e.g. -t / -type with the default being app, so you could document dancer2 gen -t app -n MyApp (leaving the clear path open for future generation of other things e.g. dancer2 gen -t plugin -n MyPlugin)?
Or would dancer2 gen ... be for generating a new app, and have new "commands" for other stuff, like .e.g dancer2 gen-plugin -n MyPlugin ?
Possibly wants a little more thought here?
|
It is certainly ambiguous when you are generating a normal app! Some examples of how other things may look with this PR: I am not opposed to separate sub commands either! I stuck with I don't think we settled anything here, but we have a better set of ideas now! What do you think? |
|
I'd be very wary of using
Hmm, we could - but would that bite us in the ass in the future if we add commands to do other things - maybe some kind of automatic "upgrade" that knows how to skim through your app and replace a deprecated way of doing something with a new way? Or a packaging/deployment shortcut, or app launching shortcut, or DB schema management, or My gut feeling so far is separate subcommands ( |
|
I'm good with this. I'll refactor the PR such that:
And this way, |
As we add new subcommands, these methods will be useful; there's no further need for the gen subcommand to keep them to itself.
cromedome
force-pushed
the
cli/rename-args
branch
from
3adb683 to
311965f
Compare
|
@bigpresh See the updated PR. I think this captures our discussion above. Thanks! |
|
Merged, thank you! |
[ BUG FIXES ]
* GH #1701: Split cookie values on & only (Yanick Champoux)
[ ENHANCEMENTS ]
* GH #530: Make data censoring configurable (Yanick Champoux, David
Precious)
* GH #850: Scaffold tutorial app; allow multiple apps to be scaffolded
in core Dancer2 (Jason A. Crome)
* GH #1512: Log hook entries as they are executed (Yanick Champoux)
* GH #1615: Remove Dancer2::Template::Simple from Dancer2 core (Jason
A. Crome)
* PR #1637: New, extendable configuration system (Mikko Koivunalho)
* GH #1723: Enable use of a different Template Toolkit base class
(Andy Beverley)
* PR #1727: Don't create CPAN package files when generating new apps
(Jason A. Crome)
* PR #1731: Retire Template::Tiny fork, use CPAN's (Jason A. Crome,
Karen Etheridge, Damien Krotkine, Yanick Champoux)
* PR #1736: Allow config system to bootstrap itself (Yanick Champoux)
* GH #1737: Add on_hook_exception for errors during hook processing
(Andy Beverley)
* PR #1739: Add source to on_hook_exception (Andy Beverley)
* PR #1742: Refactor CLI for future expansion (Jason A. Crome)
[ DOCUMENTATION ]
* GH #1342: Document skipping private methods in pod coverage tests
(Jason A. Crome)
* PR #1721: New Dancer2 docs, reorganization of all documentation;
from TPRF grant (Jason A. Crome)
* PR #1741: Cover items missed in earlier documentation branch (Jason
A. Crome)
[ DEPRECATED ]
* None
[ MISC ]
* None
An upcoming version of Dancer2 will allow you to scaffold plugins and config readers in addition to applications. As they currently stand, the existing CLI options are only meaningful to applications. This slight change makes the same options more meaningful for all types of projects.