Review: Approve
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
The proposal to merge lp:~mahfiaz/openlp/bug-933706 into lp:openlp has been
updated.
Status: Needs review = Approved
For more details, see:
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Your team
Review: Approve
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
The proposal to merge lp:~mahfiaz/openlp/bug-933706 into lp:openlp has been
updated.
Status: Approved = Merged
For more details, see:
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Your team OpenLP
Review: Needs Fixing
The copyright header is out of date, a few more names have since been added.
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113314
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list:
Review: Needs Fixing
518 - 540: Various i+1 and j+1's should be i + 1 and j + 1
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113314
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list:
mahfiaz has proposed merging lp:~mahfiaz/openlp/bug-933706 into lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
Jonathan Corwin (j-corwin)
Tim Bentley (trb143)
For more details, see:
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Adds SundayPlus importer
Review: Approve
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113465
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Review: Needs Information
Lines 205+ and 611+... couldn't these get stuck in an infinite loop?
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/111937
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list:
Superfly, *.ptf is correct. Thanks for the idea. I changed it back into two
functions again. I hope these constants found the right place.
Jonathan, you are right, I forgot to remove import of logging after I removed
the logging part.
Although lines 205 and 611 look like an infinite loop, then
Review: Needs Fixing
450, 458 not needed
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/113314
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to :
Review: Needs Fixing
Line 8 16 - You're importing logging and defining log, but I can't see where
you use it in this module
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/111937
Your team OpenLP Core is subscribed to branch lp:openlp.
mahfiaz, will you be able to look at this soon?
It's just I want the striprtf functions too, so if you're not going to look at
this soon then I would like to do so instead! Thanks.
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/111937
Your team OpenLP Core is subscribed to
92, 469: You can still separate words even with capitalized constants. e.g.
HOTKEY_TO_VERSE_TYPE
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/111791
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list:
mahfiaz has proposed merging lp:~mahfiaz/openlp/bug-933706 into lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
Tim Bentley (trb143)
For more details, see:
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/111937
Adds SundayPlus importer with new StripRtf class.
--
Review: Needs Fixing
You don't really need a whole class for two functions and a bunch of constants,
do you? If you really feel the need you can create a new file/module (rtf.py?),
but I don't see the need for a class (object-orientated vs class-orientated
programming).
Just checking, this
Review: Needs Fixing
hotkeyToVerseType is constant so should be CAPITALS and constant!
Like wise destinations etc. Even if they are in their own class.
Failed the 11th commandment about print statements!
--
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/62
Your team OpenLP
Review: Needs Fixing
You've got:
from os.path import split
...
... = split(...)
split() is also a string method. When it comes to the os.path module, we
usually just import the entire thing, because there is no benefit to only
importing a particular function, and since a number of
mahfiaz has proposed merging lp:~mahfiaz/openlp/bug-933706 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~mahfiaz/openlp/bug-933706/+merge/60
Adds SundayPlus importer with its own strip_rtf.
--
Mentioned strip_rtf is more universal than EasyWorship importer's, it supports
setting encoding using font table. I have no EasyWorship files for testing, but
maybe we could move it into songs.lib and use it for EasyWorship as well.
--
Review: Needs Fixing
Line 97 should be capitals
in parse method variables should be more meaningful in their name and correct.
byte is a character!
Strip_rtf should these not be unicode strings?
Strip_rtf should these not be constants as this is called in a loop?
Is the decode function correct?
21 matches
Mail list logo
