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

Backup and restore working for Pro5 #1088

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Alain94W
Copy link

Latest modifications for backup and restore function

@mariogrip mariogrip requested a review from NeoTheThird January 4, 2020 00:10
@NeoTheThird
Copy link
Member

sorry for the late reply. i have yet to give this a spin. thanks for your work 👍

@Alain94W
Copy link
Author

Thanks to you, I hope that it will not be too buggy for the other devices and OS ! let me know if I can do anything else!

Copy link
Member

@NeoTheThird NeoTheThird left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. As discussed on Telegram, here are some changes i would suggest. Maybe you could also look over it again to check if there's something you want to improve code-quality-wise. Else than that, we're almost good to go.

});
}

// AW : Return the total used space for system+user data on Ubuntu os only
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Return the total used space for system+user data on Ubuntu os only
// Return the total used space for system+user data on Ubuntu os only

// AW : Return the total used space for system+user data on Ubuntu os only
async function getDeviceUsedSpaceForBackup() {
// Error to handle : No such file or directory
// --output=used: No such file or directory
Copy link
Member

Choose a reason for hiding this comment

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

What is this?

Comment on lines +412 to +416
var res = await adb.shell("df -hBG / --output=used|tail -n1");
var res2 = await adb.shell("df -hBG /userdata --output=used|tail -n1");
res = parseFloat(res);
res2 = parseFloat(res2);
return res + res2;
Copy link
Member

Choose a reason for hiding this comment

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

make this one statement

return res + res2;
}

// AW : Check if /data/user-data is present and mount it if not.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Check if /data/user-data is present and mount it if not.
// Check if /data/user-data is present and mount it if not.

Comment on lines +460 to +462
}); // mount
//reject("Partition not mounted "+e);
}); //ls
Copy link
Member

Choose a reason for hiding this comment

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

remove dead comments

var bckpNames = [];
return new Promise((resolve, reject) => {
fs.readdir(directoryPath, function(err, files) {
//handling error
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//handling error

});
}

// AW : Read Backup config file
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Read Backup config file
// Read Backup config file

return JSON.parse(rawdata);
}

// AW : Modifying the maxBuffer size to handle the output of the backup (all file path backuped on the device!)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Modifying the maxBuffer size to handle the output of the backup (all file path backuped on the device!)
// Modifying the maxBuffer size to handle the output of the backup (all file path backuped on the device!)

});
}

// AW : Needed for backup
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Needed for backup
// Needed for backup

return toolpath;
}

// AW : Generate a config file for the backup created
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// AW : Generate a config file for the backup created
// Generate a config file for the backup created

@NeoTheThird
Copy link
Member

We should also already account for the fact that the installer is going to support other OSs going forward. Until we change the format, i suggest we add a property utbackup on the top level and only present the buttons if it's set. Let me know if you want to take a swing at that, or if you'd prefer me handling that.

@@ -166,6 +166,8 @@ body .vertical-centered-box .content {
z-index: -1;
}

.pull-animation,
.push-animation,
Copy link
Member

Choose a reason for hiding this comment

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

Is this an intended duplication?

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.

3 participants