Review: Needs Fixing
Hey John, a couple things.
1. You have a ton of linting issues. I started commenting, and then I realised
that it would be better to just point you to flake8. See the link at the bottom
of my comment for a quick introduction to linting and flake8.
2. You have some inconsistent indentation. Indentation in Python is very
important, so we take it seriously. Also, please make sure you are indenting
using spaces and not tabs.
3. You are not committing your code with the e-mail address associated with
Launchpad. Please can you fix that by issuing a bzr whoami "John Lines
"
4. Once you've made all your changes, and you're ready for another review, you
need to resubmit your merge proposal. Do this by clicking the "Resubmit" link
in the top right hand corner of the page.
5. If you're struggling with anything, pop into our IRC channel, there's
usually someone around who is happy to help.
https://medium.com/python-pandemonium/what-is-flake8-and-why-we-should-use-it-b89bd78073f2
Diff comments:
>
> === added file 'openlp/plugins/songs/lib/importers/singingthefaith.py'
> --- openlp/plugins/songs/lib/importers/singingthefaith.py 1970-01-01
> 00:00:00 +
> +++ openlp/plugins/songs/lib/importers/singingthefaith.py 2019-06-29
> 14:44:42 +
> @@ -0,0 +1,381 @@
> +# -*- coding: utf-8 -*-
> +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4
> softtabstop=4
> +
> +###
> +# OpenLP - Open Source Lyrics Projection
> #
> +#
> --- #
> +# Copyright (c) 2008-2019 OpenLP Developers
> #
> +#
> --- #
> +# This program is free software; you can redistribute it and/or modify it
> #
> +# under the terms of the GNU General Public License as published by the Free
> #
> +# Software Foundation; version 2 of the License.
> #
> +#
> #
> +# This program is distributed in the hope that it will be useful, but
> WITHOUT #
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> #
> +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> #
> +# more details.
> #
> +#
> #
> +# You should have received a copy of the GNU General Public License along
> #
> +# with this program; if not, write to the Free Software Foundation, Inc., 59
> #
> +# Temple Place, Suite 330, Boston, MA 02111-1307 USA
> #
> +###
> +"""
> +The :mod:`singingthefaith` module provides the functionality for importing
> songs which are
> +exported from Singing The Faith - an Authorised songbook for the Methodist
> Church of
> +Great Britain."""
> +
> +
> +import logging
> +import re
> +import os
> +from pathlib import Path
> +
> +from openlp.core.common.i18n import translate
> +from openlp.plugins.songs.lib.importers.songimport import SongImport
> +
> +log = logging.getLogger(__name__)
> +
> +
> +class SingingTheFaithImport(SongImport):
> +"""
> +Import songs exported from SingingTheFaith
> +"""
> +
> +hints_available = False
> +checks_needed = True
> +hintline = {}
> +hintfile_version = '0'
> +hint_verseOrder = ''
> +hint_songtitle = ''
> +hint_comments = ''
> +hint_ignoreIndent = False
> +
> +
> +def do_import(self):
> +"""
> +Receive a single file or a list of files to import.
> +"""
> +if not isinstance(self.import_source, list):
> +return
> +self.import_wizard.progress_bar.setMaximum(len(self.import_source))
> +for file_path in self.import_source:
> +if self.stop_import_flag:
> +return
> +with file_path.open('rt', encoding='cp1251') as song_file:
> +self.do_import_file(song_file)
> +
> +def do_import_file(self, file):
> +"""
> +Process the SingingTheFaith file - pass in a file-like object, not a
> file path.
> +"""
> +singingTheFaithVersion = 1
> +self.set_defaults()
> +# Setup variables
> +line_number = 0
> +old_indent = 0
> +chorus_indent = 5 # It might be 6, but we test for >=
We're not fond of inline comments. Rather place the comment above the line.
> +song_title = 'STF000 -'
> +song_number = '0'
> +ccli = '0'
> +current_verse = ''
> +current_verse_type = 'v'
> +current_verse_number = 1
> +has_chorus = F
