Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-10-25 Thread Raoul Snyman
Review: Needs Fixing I'd like to see an additional test, especially if it is around your code. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/192546 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://l

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-10-24 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) Raoul Snyman (raoul-snyman) Andreas Preikschat (googol) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/192546 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-16 Thread Raoul Snyman
Review: Needs Fixing Needs tests, as per Phil's remarks. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/185704 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-15 Thread Phill
All code being submitted to trunk has to have tests now. Preferably, the tests will cover your code, but its not always possible as some code needs refactoring to test properly, so you can teat other code. As long as there is a test in there. See: http://wiki.openlp.org/Development:Unit_Tests an

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-15 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) Andreas Preikschat (googol) Raoul Snyman (raoul-snyman) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/185704 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-15 Thread Dmitriy Marmyshev
What test cases? -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184942 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubsc

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-15 Thread Andreas Preikschat
Review: Needs Fixing Needs test cases -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184942 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-11 Thread Tim Bentley
Review: Approve -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184942 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubs

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-11 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Raoul Snyman (raoul-snyman) Tim Bentley (trb143) Andreas Preikschat (googol) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184942 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-11 Thread Dmitriy Marmyshev
So, now it is tested on Python3 env. :) -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184942 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@list

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-06 Thread Tim Bentley
Traceback (most recent call last): File "/usr/lib64/python3.3/logging/__init__.py", line 937, in emit msg = self.format(record) File "/usr/lib64/python3.3/logging/__init__.py", line 808, in format return fmt.format(record) File "/usr/lib64/python3.3/logging/__init__.py", line 554, in

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-06 Thread Tim Bentley
Review: Needs Fixing Cannot get this to work with images. Rename title of image and nothing happens. Needs tests -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184024 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mail

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-05 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) Andreas Preikschat (googol) Raoul Snyman (raoul-snyman) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/184024 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-04 Thread Raoul Snyman
Review: Needs Fixing 56 + if service_item[u'service_item'].is_capable(ItemCapabilities.CanEditTitle): 70 + if not self.service_items[item][u'service_item'].is_capable(ItemCapabilities.CanEditTitle): 72 + title = self.service_items[item][u'service_item'].title 76 + self.servic

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-09-03 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) Andreas Preikschat (googol) Raoul Snyman (raoul-snyman) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/183812 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title 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/~marmyshev/openlp/item_title/+merge/173606 Your team OpenLP Core is subscribed to branch lp:openlp.

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-07-11 Thread Raoul Snyman
Review: Needs Fixing On lines 73 and 74 below, where does "self.tr()" come from? We don't use it anywhere else. Also on lines 73 and 74, please take note of the String Standards on the wiki: http://wiki.openlp.org/Development:String_Standards The title of the dialog should be in title case. Ad

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-07-08 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Andreas Preikschat (googol) Tim Bentley (trb143) Raoul Snyman (raoul-snyman) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/173606 Added ability

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-07-07 Thread Raoul Snyman
Hi Dmitriy, are you going to do any more work on this? It's a pretty useful feature to have, I know I would like it see it happen. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/164174 Your team OpenLP Core is subscribed to branch lp:openlp. ___

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-05-25 Thread Tim Bentley
Review: Needs Fixing Refactored the use of title so can make things easier. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/164174 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openl

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-05-21 Thread Tim Bentley
Hold fire as I am just about to clean up titles so this will need to change. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/164174 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openl

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-05-19 Thread Raoul Snyman
Review: Needs Fixing 9 +``CanEditTitle`` 10 +The capability to allow the ServiceManager to allow the title of the item to be 11 + edited Just say: ``CanEditTitle`` The capability to edit the title of the item. -- https://code.launchpad.net/~mar

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-05-18 Thread Raoul Snyman
Dmitriy, "future" is the time which is to come, e.g. tomorrow is the future. You are talking about a "feature". -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/164174 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailin

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-05-16 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) Andreas Preikschat (googol) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/164174 Added future to rename items in ServiceMana

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-29 Thread Tim Bentley
Review: Needs Fixing 107 - 110 needs to be inside the service item some how. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158858 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~open

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-19 Thread Andreas Preikschat
I think trb143 should check it, because he knows the iternal behaviour better than me. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158858 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-19 Thread Andreas Preikschat
Review: Needs Fixing In lines 106 and 110 you missed a space: len(service_item._raw_frames) ==0: -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158858 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: htt

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-15 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158858 Added future to rename items in ServiceManager. Gives more flexability in

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-15 Thread Dmitriy Marmyshev
104-110: How this works: 1. Added 1 image, then added one or more images - the title will update to plural. 2. Added group of images, then add some more imgages - the title will be the same. 3. Added several images - the title will be plural. --- this is old behavior --- 4. Added 1 image and re

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-13 Thread Tim Bentley
Review: Needs Fixing Much better but Have issue with 104-110 please see previous comment. You are exposing the internals of a ServiceItem to a plugin and that should be bad practice. The rest looks good to me though. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158653 Your

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-12 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/158653 Added future to rename items in ServiceManager. Gives more flexability in

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-12 Thread Dmitriy Marmyshev
You were right - it is better use new capability. I wanted to add it to media and presentations plugins also, but it's too complicated for me for now. I think we can do it as future task. About adding images to service: 104-110 - I think it is correct. there is no more "else" because I dont want

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-06 Thread Tim Bentley
Review: Needs Fixing Disagree about the Capabilities. They say what you can do and are data agnostic. A new one of CanEditTitle would be fine for Images and Bibles. None of the others would need this and so what about having to add to other plugins. 75 - 82 become service_item.title = None

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-06 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/157510 Added future to rename items in ServiceManager. Gives more flexability in

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-04 Thread Dmitriy Marmyshev
38-40 - New Campability (like CanEditTitle) will need to add almost in every plugin. This is not good if you already have campability about title... code: not service_item[u'service_item'].is_capable(ItemCapabilities.HasDetailedTitleDisplay) and not service_item[u'service_item'].is_capable(Item

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-04 Thread Tim Bentley
Review: Needs Fixing 38-40 having two nots is not simple to read and does not explain what you are trying to block. An New Capability may be easier! 82-88 you are exposing internal workings of the service item in the plugins try and put this in the service item and use the registry to get the

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-04-02 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/156694 Added future to rename items in ServiceManager. Gives more flexability in

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-02-13 Thread Dmitriy Marmyshev
Yes Andreas, I'm going to chenge it. I just had no time to do this before because of ministries in church. This is good and really needed future for our church so any way i'd like to finish it. It just was not so easy as I expected. The linking of items in schedule and media base by title - was a

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-02-11 Thread Andreas Preikschat
Are going to change your code? The "Need fixing"s do not mean that we do not like your idea, it just needs fixing ;) If you need help feel free to ask. -- https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/142941 Your team OpenLP Core is subscribed to branch lp:openlp. _

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-01-11 Thread Jonathan Corwin
Also you need to bear in mind that songs and customs can be linked between the media manager and the service manager. So editing a song will sometimes also change the media manager item (depending on settings). Therefore you need to consider the consequences of changing the service item name.

Re: [Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-01-11 Thread Tim Bentley
Review: Needs Fixing Please look at the use of Title within the service item as this change is will break OpenLP. The title you are editing is not always displayed but is been used to store information about players. Media and Presentations I suggest you have a look at ServiceItem class. A

[Openlp-core] [Merge] lp:~marmyshev/openlp/item_title into lp:openlp

2013-01-11 Thread Dmitriy Marmyshev
Dmitriy Marmyshev has proposed merging lp:~marmyshev/openlp/item_title into lp:openlp. Requested reviews: OpenLP Core (openlp-core) For more details, see: https://code.launchpad.net/~marmyshev/openlp/item_title/+merge/142941 Added future to rename items in ServiceManager. Gives more flexabili