-
Notifications
You must be signed in to change notification settings - Fork 72
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
Allows to change the db with an already existing db #252
base: master
Are you sure you want to change the base?
Conversation
@@ -587,13 +592,6 @@ bool medDatabaseController::moveDatabase( QString newLocation) | |||
qDebug() << "Restarting connection..."; | |||
this->createConnection(); | |||
} | |||
|
|||
// now delete the old archive | |||
if(medStorage::removeDir(oldLocation)) |
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.
I would say you may still want to delete the old database if the folder you moved to was empty.
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.
You mean if you create a new db, you want the old db to be deleted?
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.
Yep, that would sound logical to me : if you're creating a new folder, then you wanted to move the database. If not, that means you want to reuse an older one and therefore keep the old one as well. But I'm open to discussion on this. In terms of code, it's a very easy change
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.
OK I see your point.
But can we have old db if we cannot create a new one without deleting an old one?
(In my case, I got old db because they were created when compiling medInria, MUSIC, etc.)
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.
Won't merge until resolution on this topic.
For me - deleting the old DB should be a prompt, rather than mandated. This permits us to preserve DBs if we so choose. Users won't change DBs frequently enough to be bothered by an extra step?
If this is acceptable, let me know and I'll implement the change myself.
Hola chicos, just passing through with my 2 cents (well 2 centavos, I'm in Bolivia): the whole business of moving DB when a new directory is selected seems a little silly. When the DB path is changed, medInria should either just use whatever is there as the new DB, or create a new empty one if nothing was there, but not move the whole DB to the new folder. If the user wants to move the DB to a new place, he can just use his file manager from his OS, and then update the path when opening medInria the next time. Moving files around is so prone to disaster, medInria should really stay away from that. In your case, @LoicCadour, you would just start MUSIC and MUSIC Viewer, update their DB path to the medInria one, and from then on, all updates to this DB would be shared by all. Solved. Obviously, don't open more than one of these software at once. But that's either managed by @papadop use of single-instance app he implemented a while ago, or if that mecanism is not shared across Music and medInria, maybe add a lock to the DB so that only one app can open it at the time. ...Yes, I'm still watching everything. 👀 Well, not really, I just got curious if you lot had managed a new release in my absence, and I ended up reading some pull requests. Bad habits die hard I guess ;) Hasta luego chicos! |
[DB] serie->series, and rm empty line in DB combobox
While switching between medInria, MUSIC and MUSIC Viewer on Windows (I don't think I had this behavior on Mac), I had my db location changed every time (db directory = ../organisationName/applicationName). Since my MUSIC Viewer db was new and empty, I wanted to select an already existing db (e.g medInria). But it is impossible because, from the source code, a valid db directory is an empty one.
In this PR, I try to fix that by first checking whether the selected directory contains a db file.
I also removed the "previous db deletion" because one may want to switch between db without deleting it.
Finally, I added a label warning that the changes will be effective after restart of medInria.