VSCode Extension Controller #92
Conversation
trevorNgo
requested review from
dgomezc,
KirillShchetinin,
markAtMicrosoft,
mohannad-abwah,
mvegaca,
sibille and
smmatte
as code owners
| return this.AzureCosmosDBProvider.validateCosmosDBAccountName(cosmosDBAccountName, this.usersCosmosDBSubscriptionItemCache) | ||
| .then((message) => { | ||
| if (message === undefined || message === null || message === "") { | ||
| return Promise.resolve(true); |
There was a problem hiding this comment.
To get undefined and null, you can use:
message != null
But in general, it's safer to use === or !==
There was a problem hiding this comment.
Long shot, but, message is the string? Might there be some extension method like message.IsNullOrEmpty() ?
It generally looks nicer then line of a == b
| import { AzureAuth, SubscriptionItem } from './azure-auth/azureAuth'; | ||
| import { SubscriptionError, ValidationError } from './errors'; | ||
| import { FunctionProvider } from './azure-functions/functionProvider'; | ||
| import { CosmosDBDeploy } from './azure-cosmosDB/cosmosDbModule'; | ||
| import {ReactPanel} from './reactPanel'; | ||
| export abstract class Controller { | ||
|
|
There was a problem hiding this comment.
for the purpose of stylecop - swap lines 7 & 8
| private static routingMessageReceieverDelegate = | ||
| function (message: any) { | ||
| switch (message.command) { | ||
| case "alert": |
There was a problem hiding this comment.
Don't leave just names for message.command in the code directly pls.
I'd suggest create some enum value with description of what is it for and what are the options.
- If it's not for private branch, and you want this code for master - don't leave commented lines of code. If line is not needed - remove it.
| } | ||
|
|
||
| public deployFunctionApp() { | ||
| public static deployFunctionApp() { | ||
| throw Error("undefined"); |
There was a problem hiding this comment.
What is this for? and what does "undefined" mean?
If it's not implemented (yet) - either throw "not implemented" or, actually, remove this method at all until you implement it.
Every method that is added (especially public) should provide possibility to be called and should do whatever it's supposed to do.
Also, deploy function app - what app I'm supposed to deploy, is there any parameter for the app?
Or, I assume you just put wrong modifier - should it be public?
There was a problem hiding this comment.
Good point, this function will be implemented after Redmond, our focus will be on Cosmos
| * Caching is used for performance; when displaying live check on keystroke to wizard | ||
| * | ||
| */ | ||
| private static async updateCosmosDBSubscriptionItemCache(subscriptionLabel : string): Promise<void> { |
There was a problem hiding this comment.
General question - can you explain to yourself why every single method in this file is "static"?
I'd also be curios myself.
|
Will close and remerge demo-redmond ->dev post-redmond |
Implemented controller architecture that extracts logic from react panel.