← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~smpettit/openlp/data-path into lp:openlp

 

> I've made some fast tests on Linux. In general it works. In detail there are a
> lot things to improove.
> 
> A general question (maybe I missed sone previous discussion): Why do we need
> this subfolder structure? I can understand the data directory as this allows
> us to place more directories into ~/.openlp/ , but why do you add the OpenLP
> directory into the structure? It ends up with a long directory structure
> without additional use. People should be able by their own th structure their
> directories.

I think we could/should append something like openlpdata.  If we don't append something, the data could be put into a "general" directory intermixed with other sub-folders. This would "complicate" subsequent data directory changes or possibly impact performance of OpenLP.

> 
> The label 123-126 shouldn't be shown with this text when the copy checkbox is
> not ticked. Maybe make the warning bold.

Can do. BOLD sounds good.  I even wanted to make it "red" but was told color text is not used.

> 
> 206: remove the linebreak at the end of this line. In fact: I think it's best
> to remove the whole dialog. The fact, that copying is happening while OpenLP
> ends can be put on a lable like newDataDirectoryHasFilesLabel. The people are
> still able to revert and the directory is shown in the edit line.
> 
> Never mention the full path. People don't have to be confused with the full
> path and the details wich data structure do we use. It is fine for people to
> know the base folder they selected.

Good point.  

> 
> Add labels: "Current:", "New:"

Will do

> 
> Your buttons: Use toolbuttons with tooltips as you see it in the
> defaultImageGroupBox.

I'll work on that.  Still new to GUI coding.

> 
> Changes should be applied in the save() method. If I press cancel in the
> dialog, I don't want the settings to be applied.

In my first proposal on this I was asked to move the change directory code out of the settings manager and put it in mainwindow to execute after everything has been "shutdown".  There is a "Cancel data directory change" button.  Also, if the user Cancels the dialog, the change is not applied (Lines 410-411)
 
> Check, if write access to the choosen directory is permitted.

Will do

> 
> You show lots of dialogs. Think about, which are necessary. You can maybe
> merge some of them. For example: Don't show two dialogs at program end, but
> merge them to one.

I added a dialog stating the change is about to be made in case the data is being moved to removable media.  This can take a bit of time so telling the user that something "not normal" is going to happen, and whether it was successful or not, was something I thought was needed.  

> 
> According to non-Windows systems: other OS are case sensitive. Name the "Data"
> directory without caps. If I'm not misleaded, then all* directories which
> OpenLP created from a fixed name are lower case. (* except the presentation
> program names). If we do not stick to a consistent naming we have to do all
> name comparation case insensitive.

Will do
-- 
https://code.launchpad.net/~smpettit/openlp/data-path/+merge/89612
Your team OpenLP Core is requested to review the proposed merge of lp:~smpettit/openlp/data-path into lp:openlp.


Follow ups

References