Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Online incremental megolm backups (v2)#2169

Merged
dbkr merged 39 commits into
developfrom
dbkr/e2e_backups
Nov 21, 2018
Merged

Online incremental megolm backups (v2)#2169
dbkr merged 39 commits into
developfrom
dbkr/e2e_backups

Conversation

@dbkr
Copy link
Copy Markdown
Member

@dbkr dbkr commented Sep 18, 2018

@dbkr dbkr mentioned this pull request Sep 18, 2018
8 tasks
@dbkr dbkr changed the base branch from master to develop September 18, 2018 14:10
@dbkr dbkr changed the title WIP: online incremental megolm backups (v2) Online incremental megolm backups (v2) Oct 30, 2018
* Walks the user through the process of creating an e22 key backup
* on the server.
*/
export default React.createClass({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't suggest we change this one, but surely we should be creating new stuff as ES6 at last given the language caught up (and React 16 mandates it?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess we should update the code style as I don't think we have

phase: PHASE_GENERATING,
});
// Look, work is being done!
await Promise.delay(1200);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, i'm totally missing why we'd want a pregnant pause at this point

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was me making it look like it's doing something to help the user understand the key is being generated rather than just shown to you, which it sort of des if it comes up instantly. Given it's all temporary UI I thought I'd experiment. :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. might be worth adding the rationale - i.e. to fake a generation

Comment thread src/components/views/dialogs/keybackup/RestoreKeyBackupDialog.js Outdated

_deleteBackup() {
const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog");
Modal.createTrackedDialog('Delete Backup', '', QuestionDialog, {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: doublequotes for strings, iirc? rather than flipping back and forth in the same method

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our code style says single by default so I've changed them to be consistent that way (apart from the longer, more English-y ones that are liable to contain apostrophes).

Comment thread src/components/views/settings/KeyBackupPanel.js Outdated
Comment thread src/components/views/settings/KeyBackupPanel.js
@ara4n
Copy link
Copy Markdown
Member

ara4n commented Oct 30, 2018

that's pretty epic, and lgtm other than very minor nits. can't wait to use it!

@ara4n
Copy link
Copy Markdown
Member

ara4n commented Nov 9, 2018

lgtm!

@dbkr dbkr merged commit d714176 into develop Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants