-
Notifications
You must be signed in to change notification settings - Fork 6
added (optional) backup feat for user #50
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
jmkerloch
left a comment
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.
For me there is a regression for action not related to QDT profile export.
This should be fixed.
| self.target_plugins = collect_plugin_names(self.target_qgis_ini_file) | ||
|
|
||
| def make_backup(self, profile_name: str) -> Optional[str]: | ||
| def make_backup(self, profile_name: str, backup_dir: str) -> Optional[str]: |
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.
| ###################################################################### | ||
| backup_path = Path(backup_dir) | ||
|
|
||
| if not backup_path.exists(): |
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.
If it doesn't exists it should be created. What do you think @kannes ?
| backup_path = ( | ||
| self.qdt_backup_file_widget.filePath() | ||
| ) # NEW sets backup_path from Widget | ||
| if profile_path and backup_path: # NEW check for both |
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.
See my remark for the parameter backup_path in make_backup. We should be able to left the path empty. In this case default value from profile manager will be used Path.home() / "QGIS Profile Manager Backup")
|
Thanks for your review @jmkerloch! I hope someone from us will have time to check back on it soonish. Feel welcome to change/fix things as you like! |
I added a possibility for the user to choose the path for their backup file.