Review: Needs Fixing
A few small fixes and then you should be good to go.
Diff comments:
>
> === modified file 'openlp/plugins/bibles/forms/bibleimportform.py'
> --- openlp/plugins/bibles/forms/bibleimportform.py 2016-08-29 16:11:09
> +0000
> +++ openlp/plugins/bibles/forms/bibleimportform.py 2016-12-04 03:28:05
> +0000
> @@ -717,6 +725,7 @@
> self.license_details_page.registerField('license_version',
> self.version_name_edit)
> self.license_details_page.registerField('license_copyright',
> self.copyright_edit)
> self.license_details_page.registerField('license_permissions',
> self.permissions_edit)
> + self.license_details_page.registerField("license_full_license",
> self.full_license_edit, "plainText")
>
Our standard is single quotes.
> def set_defaults(self):
> """
>
> === modified file 'openlp/plugins/bibles/forms/editbibleform.py'
> --- openlp/plugins/bibles/forms/editbibleform.py 2016-05-21 08:31:24
> +0000
> +++ openlp/plugins/bibles/forms/editbibleform.py 2016-12-04 03:28:05
> +0000
> @@ -61,10 +61,32 @@
> """
> log.debug('Load Bible')
> self.bible = bible
> -
> self.version_name_edit.setText(self.manager.get_meta_data(self.bible,
> 'name').value)
> - self.copyright_edit.setText(self.manager.get_meta_data(self.bible,
> 'copyright').value)
> - self.permissions_edit.setText(self.manager.get_meta_data(self.bible,
> 'permissions').value)
> book_name_language = self.manager.get_meta_data(self.bible,
> 'book_name_language')
> + """
> + Try loading the metadata, if the field does not exist in the
> metadata, continue executing the code,
> + missing fields will be created on "self.accept" (save).
> + """
This is not really the right way to do this. Each "get_meta_data" function
returns None if the row doesn't exist in the database, so change your
exceptions to this:
meta = self.manager.get_meta_data(self.bible, 'name')
if meta:
self.version_name_edit.setText(meta.value)
And so on and so forth. It's 3 lines each instead of 4, and it's clearer to see
what the code is doing and WHY.
> + try:
> +
> self.version_name_edit.setText(self.manager.get_meta_data(self.bible,
> 'name').value)
> + except AttributeError:
> + pass
> + try:
> +
> self.copyright_edit.setText(self.manager.get_meta_data(self.bible,
> 'copyright').value)
> + except AttributeError:
> + pass
> + try:
> +
> self.permissions_edit.setText(self.manager.get_meta_data(self.bible,
> 'permissions').value)
> + except AttributeError:
> + pass
> + try:
> +
> self.full_license_edit.setPlainText(self.manager.get_meta_data(self.bible,
> 'full_license').value)
> + except AttributeError:
> + pass
> + # Set placeholder texts for the fields.
> +
> self.version_name_edit.setPlaceholderText(UiStrings().RequiredShowInFooter)
> +
> self.copyright_edit.setPlaceholderText(UiStrings().RequiredShowInFooter)
> +
> self.permissions_edit.setPlaceholderText(UiStrings().OptionalShowInFooter)
> +
> self.full_license_edit.setPlaceholderText(UiStrings().OptionalHideInFooter)
> if book_name_language and book_name_language.value != 'None':
>
> self.language_selection_combo_box.setCurrentIndex(int(book_name_language.value)
> + 1)
> self.books = {}
>
> === modified file 'openlp/plugins/songs/songsplugin.py'
> --- openlp/plugins/songs/songsplugin.py 2016-09-19 18:51:48 +0000
> +++ openlp/plugins/songs/songsplugin.py 2016-12-04 03:28:05 +0000
> @@ -60,6 +60,7 @@
> 'songs/add song from service': True,
> 'songs/display songbar': True,
> 'songs/display songbook': False,
> + 'songs/display written by': False,
What's the previous behaviour? If previously the "Written by" used to always be
displayed, then this should be True, not False. We cannot change the default
behaviour of OpenLP from what people are expecting when we introduce a new
feature.
> 'songs/display copyright symbol': False,
> 'songs/last directory import': '',
> 'songs/last directory export': '',
--
https://code.launchpad.net/~suutari-olli/openlp/add-bible-license-field/+merge/312426
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
