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

Added ability to specify 'availableOptions' as a function in command #4966

Closed

Conversation

slindberg
Copy link

This is related to #4891/#4958 which prevents ember-cli-release from being updated.

The addon has a default set of options that is augmented with options specified in a configuration file. This is important so that the user can tailor the behavior of running ember release to individual repositories without having to remember what options to specify (which are often not the defaults). This presents a challenge because there's no initialization hook in commands in which to read the config file and override options. This PR provides the ability to specify the availableOptions property as a function that is invoked a single time in the constructor.

The upside to this approach is its simplicity and straightforwardness for most cases. The downside is that overwriting the availableOptions property with the results of the function invocation is weird, and might catch someone off guard who expects to be able to invoke the function internally later.

Alternatives:

  • Create a buildAvailableOptions hook that is a no-op by default and is called in the same place in the constructor.
  • Create a generic init hook that is called from the constructor to perform initialization tasks.

cc @rwjblue

@slindberg slindberg mentioned this pull request Oct 18, 2015
@rwjblue
Copy link
Member

rwjblue commented Oct 18, 2015

Create a buildAvailableOptions hook that is a no-op by default and is called in the same place in the constructor.

I think I would prefer this in general.

Create a generic init hook that is called from the constructor to perform initialization tasks.

This already exists. It is called by the CoreObject constructor (here is the version being used at the moment).

@slindberg
Copy link
Author

Arrrgh, I swear I tried init and it didn't work. I'm going to close this since the registerOptions method looks pretty stable and it can be used in the init hook to set available options after merging with the config.

@slindberg slindberg closed this Oct 19, 2015
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.

None yet

2 participants