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:
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
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
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 :
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
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
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 :
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
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
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.
___
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
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
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 +
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.
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.
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
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.
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:
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:
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.
--
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.
___
Mailing
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
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:
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:
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
38 matches
Mail list logo
