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

Refactor subcommands (ext.) #32

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

Conversation

ubolonton
Copy link
Collaborator

Extension of #30

Additional changes:

  • Moved commands into warp.command
  • Registered functions as commands by a decorator (not usable by apps yet, because there is currently no good way to load custom code before parsing command line options)
  • Fixed skeleton command not using "siteDir" option correctly ("twistd warp skeleton -d test" did not work, but "twistd warp -d test skeleton" did)

@phunehehe phunehehe mentioned this pull request Jan 21, 2013
@brendonh
Copy link
Owner

This looks mostly pretty good, but why is the CmdOptions class defined inside the decorator?

Also, I'm not sure that generally-applicable options (like siteDir) should actually be replicated on each command. What was wrong with how it worked before?

@ubolonton
Copy link
Collaborator Author

CmdOptions is defined inside the decorator so that it has access to the function's properties like name and argument list.

Yeah we shouldn't have these options replicated. The fix should have been removing that option from skeleton

@ubolonton
Copy link
Collaborator Author

I have tested this against the latest master.
Please merge it.

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.

3 participants