Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-11-14 Thread Tim Bentley 
Review: Approve


-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/191067
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-10-16 Thread Raoul Snyman 
Review: Approve


-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/191067
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-10-12 Thread Raoul Snyman 
Review: Approve

if your get_data method is only ever called by that class, I recommend making 
it private by adding a single underscore in front.
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/190486
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-10-09 Thread Raoul Snyman 
Review: Needs Fixing

Please don't put the helper in with the tests. Rather either put it in 
functional or a separate module called helpers.
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/189456
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-09-16 Thread Andreas Preikschat 
Review: Needs Fixing

Please use super() and please try to move the super() call to the beginning of 
the __init__
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/185651
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-09-14 Thread Phill 
Review: Needs Fixing


-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/185609
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-09-14 Thread Tim Bentley 
Review: Approve

Looks good from a clean up point of view and a simplification 
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/185651
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-08-31 Thread Tim Bentley 
Review: Resubmit

Please resubmit due to the age of this request and additionally it will need 
converting to Python 3 as on 1/9/2013 truck will be converted.
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/183034
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/song_helper into lp:openlp

2013-08-31 Thread Phill 
 Please resubmit due to the age of this request and additionally it will need
 converting to Python 3 as on 1/9/2013 truck will be converted.

Tim and co., would you mind commenting on the way I'm doing things? Not 
particularly the actual code but the concept?
-- 
https://code.launchpad.net/~phill-ridout/openlp/song_helper/+merge/183034
Your team OpenLP Core is subscribed to branch lp:openlp.

___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp