-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(sql): remove sql skip-adding-user question #1443
Conversation
xzf0587
commented
Jun 28, 2021
•
edited
Loading
edited
- remove sql skip-adding-user question.
- set default skip-adding-user value based on azureAccountProvider?.getIdentityCredentialAsync
I think it's OK for the most cases, but some corner cases may have problems, such as provisioning in CLI first and then provision in VSCode extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's OK for the most cases, but some corner cases may have problems, such as provisioning in CLI first and then provision in VSCode extension.
If developers haven't modify the skip-adding-user config in env file, leave it as default. The project will work.
Only specifying the skip-adding-user false in env file will make the cli provision failed.
it's better merge as a hotfix not a feature |
Let's add both test cases and do the verification |
Added the unit test for skip-adding-user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look good to me
} | ||
const skipAddingUser = ctx.config.get(Constants.skipAddingUser); | ||
if (skipAddingUser === undefined) { | ||
this.config.skipAddingUser = (await ctx.azureAccountProvider?.getIdentityCredentialAsync()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be simplified to :
this.config.skipAddingUser = !(await ctx.azureAccountProvider?.getIdentityCredentialAsync())