Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

Conversation

@rosen-vladimirov
Copy link
Collaborator

In case any command fails, we execute help command in order to show the help content. This leads to multiple trackings, i.e. - user executes only one command, but in Analytics we see two commands.
Instead of executing help command, introduce new method in htmlHelpService, that prints the help to the terminal and call it instead. Use the same method in the help command itself.
Rename htmlHelpService to helpService - it has been incorrectly named from the beginning.
Remove helpTextPath from staticConfig interface - this property is not used for more than 2 years.
Introduce tests for helpService - get the tests from AppBuilder CLI.

@KristianDD
Copy link
Contributor

Looks ok.

this.$logger.printMarkdown(help);
}

public async getCommandLineHelpForCommand(commandName: string): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this to be public?

In case any command fails, we execute help command in order to show the help content. This leads to multiple trackings, i.e. - user executes only one command, but in Analytics we see two commands.
Instead of executing help command, introduce new method in htmlHelpService, that prints the help to the terminal and call it instead. Use the same method in the help command itself.
Rename htmlHelpService to helpService - it has been incorrectly named from the beginning.
Remove `helpTextPath` from staticConfig interface - this property is not used for more than 2 years.
Introduce tests for `helpService` - get the tests from AppBuilder CLI.
As the injected dependency, class and interface have been renamed, rename the file as well.
@rosen-vladimirov rosen-vladimirov merged commit 06d4f68 into master Nov 7, 2017
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/help-service branch November 7, 2017 11:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants