openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #01603
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