Convert local database commands to typed commands#2189
Conversation
The local databases UI was essentially the only class which was defining methods using assignment to a class property rather than using function definitions and binding them. This switches it to use function definitions and binding, which is more consistent with the rest of the codebase.
|
|
||
| handleChooseDatabaseGithub = async ( | ||
| credentials: Credentials | undefined, | ||
| private async handleChooseDatabaseInternet(): Promise<void> { |
There was a problem hiding this comment.
Could you talk about why we have chooseDatabaseInternet and also handleChooseDatabaseInternet?
I see one includes withProgress and one doesn't, and the commands registered in extension.ts are using commandRunnerWithProgress. I wonder why we don't use commandRunner and handleChooseDatabaseInternet from extension.ts too. Is that the long term plan and this is just a stepping stone PR?
There was a problem hiding this comment.
Yes, exactly. We will convert the commands in extension.ts to use typed commands as well, including using withProgress instead of commandRunnerWithProgress. The reason for using two different methods is because for some of these the text that is shown in the progress notification is different, so we can't re-use the same method. I think it's also a somewhat clearer separation if we have separate methods which can be called publicly vs handling commands privately.
…database-ui-commands
2 participants
This converts the local databases UI to use typed commands. Please review this commit-by-commit since this also does some clean-up of the commands itself.
Checklist
ready-for-doc-reviewlabel there.