feedhenry / fh-fhc

FeedHenry CLI, the Command Line Interface to FeedHenry
Other
26 stars 66 forks source link

fix(help command): RHMAP-19895 - Fix issues and allow use fhc command… #445

Closed camilamacedo86 closed 6 years ago

camilamacedo86 commented 6 years ago

JIRA:

https://issues.jboss.org/browse/RHMAP-19895

WHAT:

WHY:

HOW:

Break changes checks:

Following the details about the break changes from 9.0.1 to 12.0.1

10.0.0

11.0.0

12.0.0

Ref: https://github.com/yargs/yargs/blob/master/CHANGELOG.md

Steps to verify it:

(To validate yargs)

Following the tests performed locally.

screen shot 2018-07-09 at 18 09 57 screen shot 2018-07-09 at 18 09 57
damienomurchu commented 6 years ago

Just verified this according to the verification steps above. All seems to function as expected. Just want to test around edge cases and will approve then if all ok :)

camilamacedo86 commented 6 years ago

@damienomurchu tks for the collaboration 👍 🥇

damienomurchu commented 6 years ago

@camilamacedo86 there seems to be a regression introduced with these changes where help output is no longer been output from the below commands:

fhc ERR! cb() never called! fhc ERR!
fhc Command executed with error ╭─ ~/work/fh-fhc ‹706f3eb*› ╰─$ fhc ping help
Usage: fhc ping --app= --env=

Options: --help Show help [boolean] --version Show version number [boolean] --app, -a Unique 24 character GUID of your cloud application. [required] --env, -e Unique 24 character GUID of the environment where this cloud application is deployed. [required]

Examples: fhc ping --app= Ping the cloud application --env=

Missing required argument: env fhc ERR! YError: Missing required argument: env fhc ERR! fhc ERR! System Linux 4.14.11-200.fc26.x86_64 fhc ERR! command "/home/damienmurphy/.nvm/versions/node/v6.9.5/bin/node" "/home/damienmurphy/.nvm/versions/node/v6.9.5/bin/fhc" "ping" "help" fhc Command executed with error


- fhc changes from this PR

╭─ ~/work/fh-fhc ‹706f3eb› ╰─$ node ./bin/fhc.js ping --help
Ping a cloud app ╭─ ~/work/fh-fhc ‹706f3eb
› ╰─$ node ./bin/fhc.js help ping
Ping a cloud app

camilamacedo86 commented 6 years ago

@damienomurchu it is not a regression it is a fix.

When the command node ./bin/fhc.js ping --help runs should show the help/desc of the ping command. In the published version it is returning the --help desc instead of the ping command desc which was fixed in this pr.

Note, How can be useful for the user run fhc command --help and receive the following/current message?


╭─ ~/work/fh-fhc ‹706f3eb*› 
╰─$ fhc ping --help                                               
Options:
  --help     Show help                                        [boolean]
  --version  Show version number                              [boolean]

fhc ERR! cb() never called!
fhc ERR!  
fhc Command executed with error

The goal of using the help command is to receive a message with the description of the command to know what does it.

$ node ./bin/fhc.js ping --help                                                                   
Ping a cloud app

c/c @davidffrench

davidffrench commented 6 years ago

@camilamacedo86 Unfortunately JIRA is currently down so I can't look at the original ticket opened by peter.

The expectation of running --help is to see what the usage and parameters should be. I would expect and a normal user would expect to see the following on a --help

Usage:
 fhc ping --app=<app> --env=<env>

Options:
  --help     Show help                                        [boolean]
  --version  Show version number                              [boolean]
  --app, -a  Unique 24 character GUID of your cloud application.
                                                             [required]
  --env, -e  Unique 24 character GUID of the environment where this
             cloud application is deployed.                  [required]
camilamacedo86 commented 6 years ago

@davidffrench and @damienomurchu thank you for the collab. I catcher what you mean. It is fixed now. Please, feel free to re-check. Following some tests.

screen shot 2018-07-10 at 14 55 51
camilamacedo86 commented 6 years ago

@damienomurchu the changes requested were made. Regards the break changes just the support of the NodeJS 4 which is deprecated that affects this project since in the project eng it was defined as >= 4.4 then it was changed and the bumped version is 6.0.0. However, note that in the reality this project has not been checked with versions lower than 6 a while and it should already be changed. Also, all analyse was added in the first comment of this PR.

Please, could you check?

camilamacedo86 commented 6 years ago

Hi folks (@damienomurchu, @davidffrench, @austincunningham)

The changes requested here were made. Please, could you check this one? Could we move forward?