← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~mjthompson/openlp/qt4.4 into lp:openlp

 

On Fri, May 07, 2010 at 09:11:23PM -0000 or thereabouts, Jonathan Corwin wrote:
> Review: Needs Fixing
> Get in before Jon... 
> 
>    if bible is not None: 
> 
> could be replaced with just:
> 
>    if bible:
>

But what if you have a Bible called ""?  I know I'm a bit of a
heretic, but my feeling is that an explicit compare with None is
sufficiently different with a pure boolean compare to do explicitly :)
But I'll go with the flow if that's what others think?
 
> Also the coding standards say inline comments should be two spaces
> from statement (although personally I'd put them on their own line
> in the except: but I don't know what the consensus is here, but
> standards do say use sparingly)
> 
>  try: # WindowsStaysOnBottomHint is not available in QT4.4
> 

Good catch - I knew when I wrote it I needed to look up what the right
thing to do was, and forgot!

Cheers,
Martin

-- 
martin@xxxxxxxxxxxxxxxxxx
  int deep_thought(void) {
    sleep (7.5e6*365*24*60*60); return 42;
  }
https://code.launchpad.net/~mjthompson/openlp/qt4.4/+merge/24931
Your team OpenLP Core is subscribed to branch lp:openlp.



References