-
Notifications
You must be signed in to change notification settings - Fork 6
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
L10n of all strings. #18
Conversation
src/commands/authorizeCommand.ts
Outdated
import { OrgUtils } from '../utils/orgUtils'; | ||
|
||
export class AuthorizeCommand { | ||
static async authorizeToOrg(): Promise<boolean> { | ||
const user = await OrgUtils.getDefaultUser(); | ||
let userAuthorized = false; | ||
await OrgUtils.getDefaultUser() |
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.
This is a strange mix of await
and then()
. Why are we mixing these two competing design patterns?
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.
Maybe I am not fully understanding the Promise() behavior then. Seems there are two patterns I can use:
try {
await somePromise()
} catch {
// it was rejected or some error occurred
}
or just chain them with the .then() and .catch()?
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.
Yes, that's correct. Whereas here we're doing await / then()
, which is mixing sync and async Promise consumption. We should do one or the other.
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.
Ok, I updated!
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.
This looks fine overall, but I want to sort the async vs. sync thing before signing off.
try { | ||
await OrgUtils.getDefaultUser(); | ||
return Promise.resolve(true); | ||
} catch { | ||
// Ask user to authorize to an org now only if not authorized yet. | ||
const result = await window.showInformationMessage( | ||
'Do you want to authorize an Org now?', | ||
{ title: 'Authorize' }, | ||
{ title: 'No' } | ||
l10n.t('Do you want to authorize an Org now?'), | ||
{ title: l10n.t('Authorize') }, | ||
{ title: l10n.t('No') } | ||
); |
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.
👍
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.
LGTM!
Ran prettier, too.
Also refactored OrgUtils to use a reject() when username is not determined.
Also fixed simple lint warnings.
@khawkins @sfdctaka @maliroteh-sf