feat: add new flag support for config commands to force profile input flag #272

kridai · 2021-07-28T07:09:36Z

Fixes

Closes: DI-424, DI-890
Introducing a new flag require-profile-input for config:set. Also updating the config:list command to show all the config properties

Testing Gist: Link

Screen Shots
config:set -- help

config:list

Checklist

I acknowledge that all my contributions will be made under the project's license
I have made a material change to the repo (functionality, testing, spelling, grammar)
I have read the Contribution Guidelines and my PR follows them
I have titled the PR appropriately
I have updated my branch with the main branch
I have added tests that prove my fix is effective or that my feature works
I have added the necessary documentation about the functionality in the appropriate .md file
I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

sonarcloud · 2021-07-29T04:48:51Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

onuzbee · 2021-07-29T05:08:10Z

CONTRIBUTING.md

+  * `make clean install`
  * `cd ..`
  * `git clone https://github.com/twilio/twilio-cli.git`
  * `cd twilio-cli`
  * In `package.json` replace `"@twilio/cli-core": "<Version Number>"` with `"@twilio/cli-core": "file:../twilio-cli-core"`
  * `make clean install`
-  * Test that everything is wired up correctuly with `./bin/run`
+  * Test that everything is wired up correctly with `./bin/run`


onuzbee · 2021-07-29T05:19:27Z

src/commands/config/list.js

+      if (this.userConfig[config] instanceof Array || this.userConfig[config] instanceof Object) {
+        return JSON.stringify(this.userConfig[config]);
+      }
+      return this.userConfig[config];
+    };


[Q] Does it have to be this verbose? From what I understand, JSON.stringify() handles Strings, undefined, null vars gracefully. I might be missing something here, just checking in. Could be simplified to :

const parseUserConfigValue = (config) => JSON.stringify(this.userConfig[config]);

onuzbee · 2021-07-29T05:21:13Z

src/commands/config/set.js

-        if (await this.isOverwrite(flag)) {
-          this.userConfig[flag] = this.userConfig.sanitize(this.flags[flag]);
+        if (await this.isOverwrite(configProperty)) {
+          this.userConfig[configProperty] = this.flags[flag];


Shouldn't we be calling the newly added sanitizeFlag here? I guess it got missed?

yes, I missed it will update

onuzbee · 2021-07-29T05:29:33Z

src/commands/config/set.js

+  sanitizeFlag(flag) {
+    return typeof flag === 'string' ? this.userConfig.sanitize(flag) : flag;
+  }
+


onuzbee · 2021-07-29T05:30:45Z

src/commands/config/set.js

+  'require-profile-input': flags.boolean({
+    description: 'Whether the profile flag for Twilio CLI commands is required or not.',
+    allowNo: true,
+  }),


[Q] Do we require default to false here?

It would change the functionality for the require-profile-input flag. Right now the we only need to pass the flag when we try to update it. Consider the scenario when the flag is set. Now if the default is set to false, whenever the config:set command is called for some other property say edge. it would try to override the require-profile-input flag to false, even if the flag is not passed.

Got it, makes sense. :)

…t command

ravali-rimmalapudi

Looks Good.

Sindhura3

LGTM, nice work!

sr010

Looks good!

* chore: Refactor profiles usage to projects (#257) * chore: Store API Keys in Config File (#259) * Save to config and not keytar * Remove secure storage references * Remove keytar load * Remove unnecessary ref to secureStorage * Correct logging * Remove dependent test failures * Add todos * chore: CLI Profile Remove - Check and Use config file before checking system keychain. (#262) * chore: CLI Profile Update - Using config file instead of the system keychain (#264) * chore: CLI Profile Update - Using config file instead of the system keychain * Added the test cases. * Addressed the review comments * Added the missing alignment. * Addressed the review comments * chore: Update profiles list (#265) * chore: Update profiles list * Addressed the comments * chore: Add profiles:port (#266) * Add profiles:port * Eslint fix * Minor edit * update test * Address review comments * Change nomenclature from porting profiles to porting keys * add space * chore: Throw alert when active profile is not set (#267) * Fetching profile by order if active profile is not set 1. Added the warning when active profile is NULL and no environment variable is set in case of profile create/update. 2. Added/updated the corresponding test cases. * Addressed the comments * chore: change post install messaging behaviour (#269) * feat: Added twilio config to list/set edge configuration (#270) * feat: Added twilio config to list/set edge configuration * Added the missing changes * Addressed the review comments. * Updated the missing messages * Updated the review comments. * Moved the common code functionality to utility. * Added the missing change. * Minor refactoring in the getFromEnvironment method. * chore - Updated package.json to point to cli-core release candidate and cli-test new release (#273) * [chore] - Updating package.json to point to cli-core release candidate and cli-test new release * Removed the cli-test version upgrade. * Added the shortform * Updated the cli-core version. * fix: Update release-feature-branch with main (#275) * update slack token * feat: add assets plugin to available plugins (#261) * [Librarian] Regenerated @ 8ec9e610ad617fae257b54113f6d7fa08c457a87 * Release 2.25.0 * chore: updating plugin-help version (#268) Co-authored-by: kridai <kgoswami@twilio.com> * [Librarian] Regenerated @ 7bffdec2c05f6a34b94cebd095be5b1b30705f5d * Release 2.26.0 * [Librarian] Regenerated @ 54ad9271a294331f8d036e4a3fc173bb98ecdb82 * Release 2.27.0 Co-authored-by: Elmer Thomas <ethomas@twilio.com> Co-authored-by: Phil Nash <pnash@twilio.com> Co-authored-by: Twilio <team_interfaces+github@twilio.com> Co-authored-by: kridai <kridaigoswami@gmail.com> Co-authored-by: kridai <kgoswami@twilio.com> * Updated the cli-core package version with latest. * [Librarian] Regenerated @ 54ad9271a294331f8d036e4a3fc173bb98ecdb82 * Release 2.27.1-rc * chore: Fix autocomplete bugs (#274) Co-authored-by: Shamant Raghav <sraghav@twilio.com> * feat: add new flag support for config commands to force profile input flag (#272) * feat: add new flag support require-profile-input for config commands * updated tests for config set and list * refactor logic for config set * nit, adding comment * changes per review comment, updating the parsing login for config list command * feature: Twilio update (#278) * SR: Oclif Update changes * SR: adding message to alert about the update * minor change Co-authored-by: Shamant Raghav <sraghav@twilio.com> Co-authored-by: Anuj Badhwar <abadhwar@twilio.com> Co-authored-by: Elmer Thomas <ethomas@twilio.com> Co-authored-by: Phil Nash <pnash@twilio.com> Co-authored-by: Twilio <team_interfaces+github@twilio.com> Co-authored-by: kridai <kridaigoswami@gmail.com> Co-authored-by: kridai <kgoswami@twilio.com> Co-authored-by: shamantraghav <87780745+shamantraghav@users.noreply.github.com> Co-authored-by: Shamant Raghav <sraghav@twilio.com>

## [2.33.0-rc.2](2.33.0-rc.1...2.33.0-rc.2) (2021-11-12) ### Library - Fixes * Hardcoding the name of the first part of the package ([#327](#327)) ([67b44cb](67b44cb)), closes [#313](#313) [#312](#312) [#318](#318) [#289](#289) [#287](#287) [#284](#284) [#286](#286) [#285](#285) [#294](#294) [#295](#295) [#293](#293) [#297](#297) [#298](#298) [#301](#301) [#304](#304) [#310](#310) [#306](#306) [#315](#315) [#316](#316) [#289](#289) [#281](#281) [#298](#298) [#287](#287) [#284](#284) [#304](#304) [#312](#312) [#286](#286) [#285](#285) [#283](#283) [#294](#294) [#285](#285) [#279](#279) [#301](#301) [#280](#280) [#313](#313) [#295](#295) [#293](#293) [#310](#310) [#318](#318) [#297](#297) [#306](#306) [#315](#315) [#316](#316) [#321](#321) * Update release feature branch ([#330](#330)) ([5878348](5878348)), closes [#257](#257) [#259](#259) [#262](#262) [#264](#264) [#265](#265) [#266](#266) [#267](#267) [#269](#269) [#270](#270) [#273](#273) [#275](#275) [#261](#261) [#268](#268) [#274](#274) [#272](#272) [#278](#278) * Updated cli-core version with the latest rc tag. ([abe116e](abe116e))

kridai added 4 commits July 28, 2021 12:20

feat: add new flag support require-profile-input for config commands

4f19291

updated tests for config set and list

089ad8b

refactor logic for config set

ea398be

nit, adding comment

dd55719

kridai requested a review from ravali-rimmalapudi July 29, 2021 04:48

kridai requested review from onuzbee, Sindhura3, shrutiburman, sr010 and childish-sambino July 29, 2021 04:49

onuzbee reviewed Jul 29, 2021

View reviewed changes

changes per review comment, updating the parsing login for config lis…

fc1008f

…t command

ravali-rimmalapudi reviewed Jul 29, 2021

View reviewed changes

Sindhura3 approved these changes Jul 30, 2021

View reviewed changes

sr010 approved these changes Jul 30, 2021

View reviewed changes

ravali-rimmalapudi approved these changes Jul 30, 2021

View reviewed changes

Merge branch 'release-feature-branch' into DI-424_force_profile-config

87b0d23

kridai merged commit 586d3fa into release-feature-branch Aug 2, 2021

kridai deleted the DI-424_force_profile-config branch August 2, 2021 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add new flag support for config commands to force profile input flag #272

feat: add new flag support for config commands to force profile input flag #272

kridai commented Jul 28, 2021 •

edited

Loading

sonarcloud bot commented Jul 29, 2021

onuzbee Jul 29, 2021

onuzbee Jul 29, 2021

onuzbee Jul 29, 2021

kridai Jul 29, 2021

onuzbee Jul 29, 2021

onuzbee Jul 29, 2021

kridai Jul 29, 2021

onuzbee Jul 29, 2021

ravali-rimmalapudi left a comment

Sindhura3 left a comment

sr010 left a comment

feat: add new flag support for config commands to force profile input flag #272

feat: add new flag support for config commands to force profile input flag #272

Conversation

kridai commented Jul 28, 2021 • edited Loading

Fixes

Checklist

sonarcloud bot commented Jul 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravali-rimmalapudi left a comment

Choose a reason for hiding this comment

Sindhura3 left a comment

Choose a reason for hiding this comment

sr010 left a comment

Choose a reason for hiding this comment

kridai commented Jul 28, 2021 •

edited

Loading