← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp

 

I think I would prefer verse_number_display rather than display_verse.

The Object name should be the same as the instance variable name

21	+        self.display_verse_check_box.setObjectName(u'verse_display_check_box')

46    Could we get a doc string

47 - 50 You might be able to do this:    self.display_verse =  bool(QtCore.Qt.Checked)

51    Some might object to the extra line on 51

Just my thoughts. I would wait and see what the other guys say. I'm no expert!

-- 
https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/179610
Your team OpenLP Core is subscribed to branch lp:openlp.


References