feature: help menu call for flag issues only, adjustment of getTokens api call#99
feature: help menu call for flag issues only, adjustment of getTokens api call#99brycentrivir wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
I have a lot of comments mentioning to remove options.file when checking for the allSeparate case (this is for imports). All this is doing is making -f take precedence over -A, which I think does make sense, so I'm thinking you should go through all these import files and re-order them so that options.file case comes before options.allSeparate, and this would achieve the same affect.
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
There was a problem hiding this comment.
This should be replaced with // code goes here, since we already handled the exit code with process.exit(1) prior to this.
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
There was a problem hiding this comment.
This should be replaced with // code goes here, since we already handled the exit code with process.exit(1) prior to this.
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
There was a problem hiding this comment.
This should be replaced with // code goes here, since we already handled the exit code with process.exit(1) prior to this.
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
There was a problem hiding this comment.
This should be replaced with // code goes here, since we already handled the exit code with process.exit(1) prior to this.
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
There was a problem hiding this comment.
This should be replaced with // code goes here, since we already handled the exit code with process.exit(1) prior to this.
| options.file && | ||
| (await getTokens(false, true, deploymentTypes)) | ||
| ) { | ||
| else if (options.individualObject && options.file) { |
There was a problem hiding this comment.
I'd change this to an if since the previous if will always exit if it's true.
|
|
||
| program | ||
| .description('Delete (social) identity providers.') | ||
| .addOption(new Option('-i, --idp-id <idp-id>', 'Id/name of a provider.')) |
| if (!options.idpId) { | ||
| printMessage( | ||
| 'Unrecognized combination of options or no options...', | ||
| 'error' | ||
| ); | ||
| program.help(); | ||
| process.exitCode = 1; | ||
| program.help(); | ||
| } |
There was a problem hiding this comment.
Delete this, it is not necessary since id is required
| !options.idpId && | ||
| !options.all && | ||
| !options.allSeparate && | ||
| !options.file |
There was a problem hiding this comment.
Missing options.file for both id and all cases
| } | ||
| // --all-separate -A | ||
| else if (options.allSeparate && !options.file && (await getTokens())) { | ||
| else if (options.allSeparate && !options.file) { |
phalestrivir
left a comment
phalestrivir
left a comment
There was a problem hiding this comment.
Accidentally submitted my first review before I finished, so here are all the other changes you need to make.
Note that you forgot to update the theme-import file. There might be other files you didn't update as well, so make sure you double check, especially if you end up rebasing with main there will likely be new commands that need to be updated as well.
| .addOption( | ||
| new Option('-i, --journey-id <journey>', 'Name of a journey/tree.') | ||
| ) |
| // .addOption( | ||
| // new Option( | ||
| // '-a, --all', | ||
| // 'Disable all the journeys/trees in a realm. Ignored with -i.' | ||
| // ) | ||
| // ) |
There was a problem hiding this comment.
I would just delete this I think since it's not implemented
| if (!options.journeyId) { | ||
| printMessage( | ||
| 'Unrecognized combination of options or no options...', | ||
| 'error' | ||
| ); | ||
| process.exitCode = 1; | ||
| program.help(); | ||
| } |
There was a problem hiding this comment.
Delete this, not necessary since id is required
|
|
||
| // disable | ||
| if (options.journeyId && (await getTokens())) { | ||
| if (options.journeyId) { |
There was a problem hiding this comment.
Delete this if statement, not necessary since id is required
| program | ||
| .description('Enable journeys/trees.') | ||
| .addOption( | ||
| new Option('-i, --journey-id <journey>', 'Name of a journey/tree.') |
| if (!options.secretstoreType && !options.secretstoreId) { | ||
| printMessage( | ||
| `'${options.secretstoreType}' does not have mappings.`, | ||
| 'Unrecognized combination of options or no options...', | ||
| 'error' | ||
| ); | ||
| process.exitCode = 1; | ||
| } else if ( | ||
| options.secretstoreId && | ||
| (await getTokens( | ||
| false, | ||
| true, | ||
| options.global ? globalDeploymentTypes : deploymentTypes | ||
| )) | ||
| program.help(); | ||
| } |
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
| if (options) { | ||
| // code goes here | ||
| } else { | ||
| process.exitCode = 1; | ||
| } |
| !options.serverId && | ||
| !options.serverUrl && | ||
| !options.all && | ||
| !options.allSeparate && | ||
| !options.file |
There was a problem hiding this comment.
Missing options.file for id, url, and all cases
| !options.serviceId && | ||
| !options.file && | ||
| !options.all && | ||
| !options.allSeparate |
There was a problem hiding this comment.
Missing options.file for id and all cases
2 participants
No description provided.