launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

09:47 < lifeless> henninge: wouldn't it be simpler for the user option parser to just store the string ?
09:47 < lifeless> henninge: thats what all the other scripts do, don't they ?
09:48 < henninge> lifeless: AFAIUI the reason behind this construct is that the option is validated during initialzation.
09:49 < lifeless> henninge: but its means you're in a transaction before you've taken out the script lock
09:49 < lifeless> henninge: this seems unwise
09:49 < henninge> lifeless: hm, true
09:49 < lifeless> probably harmless in this case
09:49 < lifeless> but I can imagine things that do lock in the db having trouble
09:50 -!- deryck [~deryck@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] has quit [Quit: deryck]
09:50 < lifeless> anyhow, I'm fine if you want to land this
09:51 < lifeless> mmm, perhaos
09:51 < lifeless> acutally no
09:51 < lifeless> it conflicts with the inifile-shutdown thing for scripts
09:52 < lifeless> here is why: option parsing is in order supplied; until we consult the ini file (found via the config) to see whether the script can run, which means we need to know the config to be using...
09:52 < lifeless> we can't be sure that the DB will even be available
09:52 < lifeless> perhaps I'm wrong
09:54 < henninge> lifeless: yes, it seems wiser to change that construct.
09:54 < henninge> even if that is a bit more work
09:55  * henninge regrets not having had a proper pre-imp discussion as he had meant to ...
09:55 < henninge> lifeless: thanks! ;-)
09:55 < lifeless> no probs!
09:55 < lifeless> I'll paste this in the review for education of anyone looking at it

-- 
https://code.launchpad.net/~henninge/launchpad/bug-823164-remove-translations-by/+merge/70961
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/bug-823164-remove-translations-by into lp:launchpad.