[08:27] hi henninge [08:28] Hey bac! ;) [08:28] henninge: so did you get UI graduated? [08:28] bac: so rockstar tells me. Is it his call alone? [08:29] henninge: no, he talked to me and sinzui. i just wasn't sure. the discussion of it in the meeting notes last week was a little anti-climatic [08:29] henninge: so, congratulations! [08:29] bac: Thank you! [08:30] henninge: thanks to you. i've updated the wikis to show it. === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:32] hi bac, do you feel like reviewing my branch? 24 lines of diff but a huge cover letter. :-) https://code.launchpad.net/~jtv/launchpad/bug-662552-defer-potmsgset-filter/+merge/39426 [10:34] jtv: ok [10:35] thanks bac [10:44] jtv: who is UCP? [10:44] Universitat Politècnica de Catalunya [10:45] Which I believe means Universidad Politécnica de Cataluña. [10:45] Great database professor there. [10:48] Made things really accessible, and got his team to implement a real-world innovation in postgres instead of messing about with theory and simulations. [10:49] that's cool [10:49] thanks for the branch and explanation. [10:51] thanks for the review—"mysterious" is about right. :-) [12:07] hey jtv, I am unsure atm: Will this code also return existing languages or throw an exception if the code exists? [12:07] http://paste.ubuntu.com/520708/ [12:08] from LangugeSet [12:08] henninge: well it constructs a Language. [12:08] So there's your answer. :) [12:08] jtv: you are not helping [12:09] It'll create a new Language object, and if one of the same code or englishname already existed, then you'll get a unique violation when it gets flushed to the database. [12:09] jtv: now you are helping ;-) [12:09] I suspected that but forgot... [12:09] * jtv regrets it already :) [12:09] It's just a constructor. Constructors don't look for existing objects. === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara-afk is now known as matsubara === henninge changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:55] jtv: It's a pitty you just eod'ed. You would have liked this one ... ;-) [12:56] ? [12:56] https://code.edge.launchpad.net/~henninge/launchpad/devel-productseries-views-tests/+merge/39437 [12:56] * jtv checks channel name [12:56] But I will go to lunch myself now ... [12:56] yes, I just eod'ed. :) [12:56] :-P [12:59] https://code.launchpad.net/~lifeless/launchpad/zope.testing/+merge/39423 === jtv is now known as jtv-afk [14:17] henninge: someone who probably isn't here right now just came by and spontaneously reviewed your branch [14:31] jtv-afk: that is awfully nice of him (or her?) ! === henninge changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === adeuring1 is now known as adeuring [15:12] is someone available to do a ui review on https://code.edge.launchpad.net/~jcsackett/launchpad/hidden-project-configuration-636000/+merge/39306 [15:16] as long as we're asking for reviews, i need a code review for https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/enhanced-consumer/ (writing mp now) [15:16] jcsackett, i'd swap reviews with you if i were rated for ui reviews [15:16] leonardr: i'd take you on that if i were a reviewer. :-P [15:17] like two ships passing in the night [15:22] * jcsackett laughs. [15:27] jcsackett: I'll take it. [15:27] henninge: thanks! [15:31] salgado, maybe you want to take a minute and review https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/enhanced-consumer/+merge/39447 ? [15:35] rockstar: when you get up, can you please add me to launchpad-ui-reviewers? ;-) === Ursinha is now known as Ursinha-afk [15:36] henninge, ah! [15:36] Hi rockstar! ;) [15:36] henninge, hello sir. [15:37] henninge, done. [15:38] rockstar: great, thank you! Now I can claime that review ... [15:39] s/claime/claim/ [15:40] jcsackett: That looks very good, thank you! I just wonder if "Configuration links" is the best title. [15:40] jcsackett: how about "Configuration steps"? [15:40] not sure; that implies a sequence, and you don't really have to do any of them in sequence. [15:40] in all truth, we just really *want* people to set things up. if they don't care they don't have to do any of it. [15:41] "Configuration details" [15:41] henninge: yeah, i can go with that. [15:41] It's just that "links" are all over the place on a webpage [15:41] I was also thinking "Configure services" [15:41] Or that. [15:41] preference, oh ui reviewer? :-) [15:42] i'm good with either, personally. [15:42] does one seem better or more in keeping with convention? [15:43] what was it you copied from? [15:45] ah, "extra options" [15:45] May "Configuration options"? [15:46] that works. and that does seem to follow the convention. [15:46] i'll make that change push it up. thanks, henninge. [15:46] jcsackett: cool, let's use that. [15:46] jcsackett: there is something else [15:46] I just ran the branch and it looks different to the screenshot. [15:46] s/to/from/ ? [15:46] * jcsackett looks alarmed. [15:47] There is the blank line missing after "Configuration links" [15:47] ah, yes. i killed the
from the code review and didn't update the screenshots. [15:47] sorry. [15:47] you prefer with the blank line? [15:47] i wasn't sure either way to go on that. [15:48] Actually, that was a bit too much. Could we have half a line? [15:48] * henninge hacks === Ursinha-afk is now known as Ursinha [15:50] henninge: i'm good with half a line, but i'll confess my css-fu is weak. [15:54] jcsackett: turn "legend" into a div and give it a "margin-bottom: 0.5em;" . That looks fine. [15:55] henninge: dig. will do. === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha [15:56] jcsackett: you will have to script your js code because this breaks it. [15:56] s/script/fix/ [15:57] henninge: yeah, i think that javascript should just be #legend not span#legend anyway. [15:57] jcsackett: or maybe just use a class? [15:57] henninge: your right. ".legend" it is. [15:58] jcsackett: call it ".configuration_optoins" [15:58] henninge: okay. [15:59] although ... [16:00] yeah, do that. I think it's better to give such specific classes specific names. [16:01] jcsackett: .config-options [16:02] last word, promised ... ;-) [16:02] r=me [16:02] thanks henninge. i'll make those changes. :-) === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-lunch === matsubara-lunch is now known as matsubara === jcsackett|afk is now known as jcsackett [20:01] Edwin: free for a review? https://code.launchpad.net/~jtv/launchpad/bug-662552-get-tm-or-dummy/+merge/39467 [20:03] A lot of it is lint. [20:10] jtv: I have a branch too, shall I add you to queue? [20:10] StevenK: yes please === StevenK changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [jtv, StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === noodles775 changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [jtv, StevenK, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:20] Hi Edwin-afk, could you please review the following when you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/627608-p3a-token-race/+merge/39469 [20:20] jtv: sorry, I didn't see your comment. I'll start reviewing it now. [20:20] EdwinGrubbs: cool, thanks [20:20] (one-line change with a new test) [20:21] noodles775: sure. I guess I'll look at that first then. [20:21] EdwinGrubbs: thanks! [20:25] noodles775: what happens if the scripts fails to run? For example, if it can't connect to the database but the next minute it runs fine. [20:27] EdwinGrubbs: ah, you mean if date_started *is* set, even though it didn't complete... good point. [20:29] I'll check script-activity and ensure we have a successful record. [20:30] EdwinGrubbs: OK, I need to update to ensure date_completed is not null in the query. I'll update the test, make sure it fails and fix. [20:33] Edwin: pls ad https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/enhanced-consumer/+merge/39447 to the queue? [20:33] EdwinGrubbs: actually, afaics, script activity is only ever recorded on success (IScriptActivitySet has only a recordSuccess method). [20:33] EdwinGrubbs: Assuming that's true, the current code is fine as is. [20:40] noodles775: ok. Is the htaccess file the only thing that this script updates, so you are completely dependent on the ArchiveAuthToken.date_created and the last_success.date_started running on machines whose clocks are in sync? [20:43] EdwinGrubbs: yes (actually the htpasswd). And again, that's a good point about needing to be in sync. Thinking. [20:44] oh eh, i just reviewed that branch :-o [20:46] Thanks mwhudson - do you know if the issue Edwin mentiones above could be a problem? (ie. the token is created on an app server, but the script activity will be on germanium). Can we trust the timestamps? [20:46] noodles775: i *think* i've asked is about this before, and we can rely on ntp running [20:46] noodles775: you might want to check though :-) [20:47] * noodles775 checks [20:47] noodles775: there is also a chance that all times will be from the db [20:48] mwhudson: date_started and date_completed are required args to IScriptActivitySet.recordSuccess, so I don't think they will be set by the db. [20:48] yeah, indeed [20:49] noodles775: you _could_ probably pass UTC_NOW for date_completed [20:49] but we don't [20:49] mwhudson: yeah, and we're checking date_started here. [20:49] also true [20:50] noodles775: i guess a fudge would be to check all tokens created >= date_started - 1 [20:50] second [20:53] noodles775: I think mwhudson fudge factor is reasonable. I'll let you two decide on the final implementation and start reviewing something else. [20:54] EdwinGrubbs: so we can assume NTP is running (ie. minimal skew), but I'll add a 1-second.... [20:54] :) [20:54] Thankyou EdwinGrubbs. [21:00] EdwinGrubbs: Are you still reviewing [21:00] ? [21:01] StevenK: yes, I'll get to yours after jtv's. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [jtv, StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [21:09] EdwinGrubbs: Excellent, thank you. I have two. :-) === matsubara is now known as matsubara-afk === Ursinha is now known as Ursinha-bbl [21:52] jtv: review sent [21:52] thx === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [StevenK, leonardr, StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [21:55] StevenK: which branch would you like me to review first. [22:00] EdwinGrubbs: The qafail [22:13] EdwinGrubbs: I'm definitely eod, but just put up another one: https://code.edge.launchpad.net/~jtv/launchpad/bug-662552-view-permissions/+merge/39481 [22:14] jtv: I doubt that I will have time to get to it. [22:14] no worries [22:48] leonardr: wouldn't you want the OAuthAuthorizer.user_agent_params to include the oauth_consumer, especially if the application_name isn't set? [22:49] OAuthAuthorizer: application_name defaults to oauth_consumer [22:49] ok, I missed that [22:50] maybe we also want the oauth_consumer, i dunno. gary, maybe you can weigh in when you get a minute? (poss. in comments on https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/enhanced-consumer/+merge/39447) [22:52] looking [22:53] EdwinGrubbs: Thank you for the review! [22:54] leonardr: it looks like you have a merge conflict in src/lazr/restfulclient/docs/operations.txt, although it will be trivial to fix. [22:55] ok [22:57] argh must run [22:58] leonardr: I'm sorry, I have to run. Do you need me to check back in later, or is it alright? [22:58] gary, it's ok [22:58] ok thanks [22:58] g'night [23:08] leonardr: i added support for qastaging to launchpadlib if you would like to take a look: https://code.edge.launchpad.net/~jcsackett/launchpadlib/add-qastaging-to-uris/+merge/39483 [23:09] having to manually throw in the url when qaing was bugging me. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [23:11] StevenK: I'll finish your other branch later tonight. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [23:11] EdwinGrubbs: Excellent, thank you [23:51] jcsackett, i'll take a look tomorrow. remind me if i forget