search

webosose / ares-cli

ares-cli is a command-line interface(CLI) for webOS
Apache License 2.0
38 stars 17 forks source link

Fixed browser invocation #48

Closed mariotaku closed 3 years ago

mariotaku commented 3 years ago

If LG_WEBOS_CUSTOM_SDK_HOME has set, ares-inspect.cmd --app com.company.package --open will not open browser properly. This PR fixed this issue.

YoungeunKim commented 3 years ago

@mariotaku, Thanks for your PR. Could you update your PR according to ares-cli guide? https://github.com/webosose/ares-cli#contributing

  1. I hope your commit will be merged into "develop" branch. Please work your RP on ares-cli's "develop" branch (not "main").
  2. Write "Commit Message"?
mariotaku commented 3 years ago

Thank you for your reply. I have updated base and commit message.

YoungeunKim commented 3 years ago

code 👍 verified 👍

@mariotaku , I reviewed your PR and it looks good to me. But to merge the PR, should get @ssuminahn's review. She's on vacation now, so she might give a reply later this week.

@ssuminahn , I verified "CLI unit test" on ose target and eslint" instead of @mariotaku. I updated @mariotaku 's commit message like below. Please review this commit.

============================================================ Fixed browser invocation

:Release Notes: Fixed callback function arguments for getEnvValue("BROWSER", ...)

:Detailed Notes: Currently, callback of getEnvValue() in _findSdkEnv() has 1 arguments: function(browserPath) while it should be 2: function(err, browserPath). This commit adds proper arguments so when using ares-inspect.js --app com.company.package --open command, it will open specified browser properly instead of system default.

:Testing Performed:

  1. Pass unit test on ose target
  2. Pass ESlint
  3. Check below commands and results a. Set $LG_WEBOS_CUSTOM_SDK_HOME, which is used in ares-cli/files/conf/sdk.json b. Install com.domain.app(web app) to target c. $LG_WEBOS_CUSTOM_SDK_HOME/IDE/chromium/chrome should be executed when using 'ares-inspect com.domain.app -o' command

:Issues Addressed: [WRN-2517] Fixed browser invocation

=============================================================

ssuminahn commented 3 years ago

@mariotaku, @YoungeunKim Sorry. There was a part I missed in the commit log, so I corrected that part and reflected it to develop branch.