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

[mod] display backup location #474

Open
wants to merge 1 commit into
base: stretch-unstable
from

Conversation

Projects
None yet
4 participants
@Psycojoker
Copy link
Member

Psycojoker commented May 14, 2018

Otherwise people don't know where to find it (had the case on the dev chan).

We should always give this kind of "obvious when you know it" information.

Microdecision, waiting a bit in case of.

@@ -80,7 +80,7 @@
"backup_cleaning_failed": "Unable to clean-up the temporary backup directory",
"backup_copying_to_organize_the_archive": "Copying {size:s}MB to organize the archive",
"backup_couldnt_bind": "Couldn't bind {src:s} to {dest:s}.",
"backup_created": "Backup created",
"backup_created": "Backup created and stored in /home/yunohost.backup/archives under the name {}",

This comment has been minimized.

@JimboJoe

JimboJoe May 14, 2018

Member

Instead of having this directory in the locale constants, shouldn't we pass ARCHIVES_PATH as a parameter to the logging instruction?
One could imagine being able to set this path to somewhere else in the future.

This comment has been minimized.

@alexAubin

alexAubin May 14, 2018

Member

In fact I wonder if the actual location of the backup isn't backup-method dependent ... for instance if you use a remote borg backup method, it won't be at all in this location.

Maybe making this message backup-method independent is complicated, so maybe a simple solution would be something like

if backup_method == "tar":
    ... display this message ...

This comment has been minimized.

@zamentur

zamentur May 14, 2018

Contributor

Some options of yunohost backup create let you choose the destination.

This comment has been minimized.

@Psycojoker

Psycojoker May 14, 2018

Member

I guess it's the job of the method to provide this information then because the manager doesn't have it.

The code doesn't look ready for that from here but it's very dense and I haven't took the time to dig in it.

@alexAubin alexAubin added this to the 3.x milestone Jun 13, 2018

@alexAubin alexAubin changed the base branch from unstable to stretch-unstable Jun 17, 2018

@alexAubin alexAubin added the inactive label Dec 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment