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

Create new command with class constructor #1339

Closed
wants to merge 1 commit into from
Closed

Create new command with class constructor #1339

wants to merge 1 commit into from

Conversation

cravler
Copy link
Contributor

@cravler cravler commented Aug 29, 2020

No description provided.

@shadowspawn shadowspawn added the semver: major Releasing requires a major version bump, not backwards compatible label Aug 31, 2020
@shadowspawn
Copy link
Collaborator

After some more thinking, perhaps not.

This works nicely if the subclass keeps the same signature for the constructor, which will usually be the case. However, the subclass may ignore the name or require additional parameters in its constructor.

Expecting the subclass to define createCommand is simplistic, but means the calling convention for creating subcommand is explicit. A single parameter which is the command name. The subclass can do what wants for its constructor.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Aug 31, 2020

Also, the return type of createCommand gets used as the return type of .command() for Typescript, a suggestion from:
#1191

@cravler
Copy link
Contributor Author

cravler commented Aug 31, 2020

agree, subclass can overide this method, if needed

@cravler cravler closed this Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: major Releasing requires a major version bump, not backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants