Skip to content
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

feat(xo-web/Backup NG/logs): display the export type #3023

Merged
merged 7 commits into from Jun 7, 2018

Conversation

badrAZ
Copy link
Contributor

@badrAZ badrAZ commented Jun 4, 2018

See #2711

Check list

Check items when done or if not relevant

  • PR reference the relevant issue (e.g. Fixes #007)
  • if UI changes, a screenshot has been added to the PR
  • CHANGELOG updated
  • documentation updated

Process

  1. create a PR as soon as possible
  2. mark it as WiP: (Work in Progress) if not ready to be merged
  3. when you want a review, add a reviewer
  4. if necessary, update your PR, and readd a reviewer

List of packages to release

No need to mention xo-server and xo-web.

Screenshots

delta
full

@badrAZ badrAZ self-assigned this Jun 4, 2018
@badrAZ badrAZ requested a review from julien-f June 4, 2018 13:00
@julien-f julien-f mentioned this pull request Jun 4, 2018
40 tasks
@badrAZ badrAZ changed the title [WiP] feat(xo-web/Backup NG/logs): display the export mode feat(xo-web/Backup NG/logs): display the export mode Jun 4, 2018
Copy link
Member

@julien-f julien-f left a comment

Choose a reason for hiding this comment

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

That's not what we want here, we want to display, in delta mode, whether the export was a full or a delta.

).then(result => ({
size: sumBy(result, 'size'),
isFull: some(result, { isDelta: false }),
}))
Copy link
Member

Choose a reason for hiding this comment

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

This should be implemented for CR as well.

IMHO, you should do it before the waitAll.

@@ -1018,12 +1018,14 @@ export default class BackupNg {
}
})()

const getIsDelta = vdi => vdi.other_config['xo:base_delta'] !== undefined
Copy link
Member

Choose a reason for hiding this comment

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

This function is not a closure, move it outside of this scope.

@@ -1018,12 +1018,14 @@ export default class BackupNg {
}
})()

const getIsDelta = vdi => vdi.other_config['xo:base_delta'] !== undefined
const isFull = some(deltaExport.vdis, vdi => !getIsDelta(vdi))
Copy link
Member

Choose a reason for hiding this comment

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

Avoid creating a wrapper around getIsDelta.

@badrAZ badrAZ requested a review from julien-f June 7, 2018 10:09
@julien-f
Copy link
Member

julien-f commented Jun 7, 2018

Please check all boxes if the appropriate action is done or if not relevant.

@badrAZ badrAZ changed the title feat(xo-web/Backup NG/logs): display the export mode feat(xo-web/Backup NG/logs): display the export type Jun 7, 2018
@julien-f julien-f merged commit a81f0b9 into master Jun 7, 2018
@julien-f julien-f deleted the displayExportType branch June 7, 2018 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants