← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~sam92/openlp/author-types into lp:openlp

 

Review: Needs Fixing

200	+ #These types are defined by OpenLyrics: http://openlyrics.info/dataformat.html#authors
201	+ TYPE_WORDS = 'words'
202	+ TYPE_MUSIC = 'music'
203	+ TYPE_TRANSLATION = 'translation'
204	+ Types = {
205	+ TYPE_WORDS: translate('OpenLP.Ui', 'Words'),
206	+ TYPE_MUSIC: translate('OpenLP.Ui', 'Music'),
207	+ TYPE_TRANSLATION: translate('OpenLP.Ui', 'Translation')
208	+ }

The Author model should not contains this. This is an enumeration, and should be in it's own AuthorType enumeration class. Also, because they are class members and not constants, they should be in PascalCase not UPPERCASE.

278	+ :return List of all authors (only required for initial song generation)

Your ":return:" is missing a second colon.
-- 
https://code.launchpad.net/~sam92/openlp/author-types/+merge/213297
Your team OpenLP Core is subscribed to branch lp:openlp.


References