← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~crichter/openlp/testing into lp:openlp

 

Database options have been talked about but tend to focus on different RDBMS support - hadn't thought about the simplest case of just externalising the standard sqlite.  Looks good.

General comments:
* I would say though that song DB options should be on the songs tab though not general.

* If you're going to do the songs DB you should do the bibles DB at the same time (on the bibles tab)

Code comments:
* Double space to single space after commas needed: 82, 84, 102, 103

* Get into the habit of a matching settings.endGroup() for each settings.beginGroup() and put any settings.value().toString() in a unicode() wrapper as we keep it all unicode internally.

* Did something fail to need the str() for the path?  Unless it doesn't work I'd keep the consistency and s/str/unicode/
-- 
https://code.launchpad.net/~crichter/openlp/testing/+merge/25728
Your team OpenLP Core is requested to review the proposed merge of lp:~crichter/openlp/testing into lp:openlp.



References