/srv/irclogs.ubuntu.com/2010/01/27/#launchpad-reviews.txt

bacsalgado-afk: i tried to review your branch but didn't get far.  i tried running the branch referenced in the MP but +login-openid does not work on that branch.  I then tried grabbing your loom branch but got bzr errors.00:01
salgado-afkbac, you should be able to try it out by merging from bzr+ssh://bazaar.launchpad.net/~salgado/launchpad/lp-as-openid-rp-for-ec200:03
salgado-afkin that branch the +login page uses OpenID, so +login-openid doesn't exist there00:04
bacsalgado-afk: got the branch00:06
bacsalgado-afk: running the new branch i still don't see +login-openid00:07
bacsalgado-afk: so, i'm confused as to what your two different branches do.00:08
salgado-afkbac, +login-openid doesn't exist anymore -- in that branch I've already switched the +login page to use OpenID00:08
=== salgado-afk is now known as salgado
salgadothe branch you're reviewing just adds the OpenID provider, and the other one changes +login to use the OpenID provider00:09
bacsalgado: and neither support the URL you listed in the MP -- +login-openid?00:10
salgadobac, the latter one used to, but I moved it over the existing +login00:11
bacsalgado: ok, i think i understand now.00:11
bac+login on the branch you posted a second ago tries to work.  i need to add testopenid.lp.dev to /etc/hosts now to see it really work00:12
salgadoyeah, forgot to mention that, sorry00:12
bacno biggie00:18
bacsalgado: missing cert00:22
=== jamalta is now known as jamalta-afk
=== jtv changed the topic of #launchpad-reviews to: on-call: jtv || reviewing: - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
jtvWho's free to review an RC/CP branch?  https://code.edge.launchpad.net/~jtv/launchpad/bug-512698/+merge/1812408:27
jtvflacoste, are you free to review a re-roll candidate?  It's just backing out another branch.  https://code.edge.launchpad.net/~jtv/launchpad/bug-512698/+merge/1812408:42
jtvI've never backed out a branch before, so would like to be sure I get it right.08:42
flacostejtv: looking08:46
jtvthanks!08:47
flacostejtv: looks good08:50
jtvflacoste: thanks...  I can't explain the conflict, so you'll notice that's the one part I didn't back out.  Seemed safer; somebody else may have started using the same JS module.08:56
=== matsubara-afk is now known as matsubara
=== henninge is now known as henninge-lunch
=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
=== jamalta-afk is now known as jamalta
=== henninge-lunch is now known as henninge
al-maisanjtv: hello there! Are you still reviewing?14:25
=== matsubara_ is now known as matsubara
=== al-maisan changed the topic of #launchpad-reviews to: on-call: jtv || reviewing: - || queue [al-maisan<http://tinyurl.com/yag7j48>] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [al-maisan<http://tinyurl.com/yag7j48>] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [al-maisan<http://tinyurl.com/yag7j48>] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== allenap` changed the topic of #launchpad-reviews to: on-call: Edwin, allenap || reviewing: -, - || queue [al-maisan<http://tinyurl.com/yag7j48>] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== beuno_ is now known as beuno
=== matsubara is now known as matsubara-lunch
=== salgado is now known as salgado-lunch
=== deryck is now known as deryck[lunch]
al-maisanhello EdwinGrubbs or allenap : would either of you be willing to review the branch listed in the topic?16:15
allenapal-maisan: I'll do it.16:15
=== beuno is now known as beuno-lunch
=== allenap changed the topic of #launchpad-reviews to: on-call: Edwin, allenap || reviewing: -, al-maisan<http://tinyurl.com/yag7j48> || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanallenap: thank you!16:15
=== salgado-lunch is now known as salgado
al-maisanback in a few minutes..16:25
=== jamalta changed the topic of #launchpad-reviews to: on-call: Edwin, allenap || reviewing: -, al-maisan<http://tinyurl.com/yag7j48> || queue [jamalta] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
jamaltaEdwinGrubbs, allenap: Let me know when you have a chance, just want to have a short pre-implementation chat about a trivial bug16:27
EdwinGrubbsjamalta: I'm available16:27
jamaltaEdwinGrubbs: I'm looking at bug #11860916:28
mupBug #118609: "List related bugs" doesn't work on hosts other than bugs.launchpad.net <trivial> <ui> <Launchpad Bugs:Confirmed> <https://launchpad.net/bugs/118609>16:28
jamaltaFirst, I wanted to see what the suggested implementation would be. Adding +bugs to the link works as expected, but would that be the best solution? Or should the link go to the bugs sub-domain instead?16:29
* EdwinGrubbs checking16:29
jamaltaSecondly, would a test for that bug just check if the link for "List related bugs" is pointing to +bugs or the bugs subdomain (depending on the implementation taken)?16:31
jamaltaOnly issue I see with using +bugs is that the link in the side menu is not active16:32
EdwinGrubbsjamalta: hmm, it seems that the root of the problem is that going to http://edge.launchpad.net/~bjornt/+assignedbugs does not redirect you to bugs.edge.launchpad.net so the canonical_url() creates a link to edge.launchpad.net instead of bugs.edge.launchpad.net16:36
jamaltaEdwinGrubbs: So the solution should be to instead redirect the original URL to bugs.launchpad.net?16:37
jamaltaI had skipped that as a solution as it resolved correctly :) But seems fair enough16:37
EdwinGrubbsjamalta: I think so. I'm looking up where the link is defined now.16:38
=== matsubara-lunch is now known as matsubara
jamaltaEdwinGrubbs: Well the sidebar menu is in lib/lp/registry/browser/person.py16:39
jamaltaBut the only way I know to get to that page is by clicking the "Bugs" tab which correctly moves you to bugs.launchpad.net, in which case all links will work correctly.16:40
jamaltaI was assuming that the use-case would be if the user typed the URL manually instead of following a link, although I could be very wrong.16:40
jtval-maisan: hi! sorry, long break, but yes.16:40
al-maisanjtv: no problem, allenap was so kind to take a look at the branch in question.16:41
jtvallenap: sorry for leaving you alone for so long.  There was nothing in my earlier day... what needs doing?16:41
al-maisanjtv: do you mind if I review yours tomorrow morning .. my day is drawing to a close.16:41
jtval-maisan: not at all!16:42
al-maisanjtv: thanks!16:42
allenapjtv: Don't worry, I started after Edwin today; I was out this morning.16:42
EdwinGrubbsjamalta: it looks like the best fix will be to change Link('', text, summary) to Link('', site='bugs', text, summary)16:42
jamaltaEdwinGrubbs: Ah, okay16:43
=== beuno-lunch is now known as beuno
jamaltaEdwinGrubbs: That makes a lot of sense, I should add site='bugs' to all of the bugs link, though, right?16:45
jamaltaAll the links within that class16:46
=== deryck[lunch] is now known as deryck
EdwinGrubbsjamalta: It's not absolutely necessary, since those urls don't overlap with urls in other domains, but it would make it more consistent.16:47
jamaltaEdwinGrubbs: Right, that's why I wanted to consider that16:48
jamaltaEdwinGrubbs: Last question would be about testing16:49
jamaltaI'm assuming I need to write a test for this bug, right?16:50
jamaltaIf so, should I simply pull out the link and make sure the URL is "bugs.launchpad.dev..."?16:50
al-maisanallenap: sorry for bothering you but I corrected two comments; this may make it easier to read the branch: http://pastebin.ubuntu.com/363970/16:50
jamaltaI can't find any tests that deal with that link specifically16:50
allenapal-maisan: Cool, thanks for letting me know.16:52
EdwinGrubbsjamalta: that should be sufficient. You can add that test to lib/lp/registry/stories/xx-person-bugs.txt16:52
jamaltaEdwinGrubbs: thanks so much! I'll get working on that then :)16:52
allenapbarry: Out of interest, do you know if argument unpacking - i.e. def foo(a (b, c)): ... - is deprecated, generally frowned upon, or jolly good stuff?17:00
barryallenap: generally frowned upon, surprising to many users, and (not sure about this) maybe gone in python 3?17:01
allenapbarry: Cool, thanks.17:01
barrynp!17:02
allenapbarry: Yes, actually, it's a SyntaxError in 3.17:02
barryallenap: cool, that's what i thought.  thanks for checking it!17:02
=== EdwinGrubbs is now known as Edwin-lunch
jamaltaallenap: since Edwin is now out to lunch, would you care to review an MP for me? It'll be fun, I promise! :)17:36
allenapjamalta: I'd love to, but I won't get to it until tomorrow; it's nearly my end-of-day here. I can take it, or you could ask tomorrow's OCR.17:37
jamaltaallenap: alright that sounds good.. thanks :)17:39
allenapjamalta: Which one? OCR or me? :)17:39
jamaltaallenap: waiting for another reviewer17:41
jamaltaor waiting for you17:41
jamaltawhichever comes first :)17:41
jamaltaI have no problem waiting, heh17:41
allenapjamalta: Okay, cool. Just to be clear, it's for bug 118609 right?17:42
mupBug #118609: "List related bugs" doesn't work on hosts other than bugs.launchpad.net <trivial> <ui> <Launchpad Bugs:Confirmed for jamalta> <https://launchpad.net/bugs/118609>17:42
jamaltaallenap: correct17:42
allenapjamalta: Actually, it's short, I'll do it now.17:43
al-maisanallenap: are you done with the other branch by any chance?17:44
jamaltaallenap: Alright, thanks!17:44
jamaltaIt is pretty short, maybe 10 lines changed ;)17:44
allenapal-maisan: I hope to have it done in the next 15 minutes.17:47
allenapjamalta: Do you need someone to land that branch for you?17:47
al-maisanallenap: great, thanks!17:47
jamaltaallenap: well, yeah i guess so :)17:48
jamaltai always get confused at that question.. 17:48
jamaltado you ask because some people land their own branches?17:48
jamaltai'll fix the description in the test, thanks17:48
allenapjamalta: Well, only some people have permission to submit the branches to PQM, which is what actually lands the branches on devel. I can land it for you. Can you either email me when it's ready, or ping me in the morning?17:49
jamaltaallenap: ah ok, i get it17:49
jamaltanow is ok with me, as long as you have time for it17:50
jamaltai fixed the doctest per your suggestion17:51
=== mrevell is now known as mrevell-dinner
allenapal-maisan: Wow, your branch has a lot in it. I'm not finished yet, but http://pastebin.ubuntu.com/364018/ has my review so far. I've just got to TestMultiArchJobDelayEstimation, and I'll finish it in the morning.18:09
al-maisanallenap: OK .. that's fine .. have a nice evening.18:10
allenapal-maisan: And you, and sorry for not getting it all done.18:10
al-maisanallenap: no problem .. I understand, it's quite a big branch.18:10
leonardredwin or allenap, can you review https://code.edge.launchpad.net/~leonardr/lazr.restful/generate-multiversion-collections/+merge/18153 ?18:14
=== sinzui changed the topic of #launchpad-reviews to: on-call: Edwin, allenap || reviewing: -, al-maisan<http://tinyurl.com/yag7j48> || queue [sinzui, jamalta] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuiEdwin-lunch: allenap: I am jumping the queue because I have a branch that address the timeouts when creating a release19:34
=== Edwin-lunch is now known as EdwinGrubbs
EdwinGrubbssinzui: is it the release-timeout-bug-513321 you want me to review19:50
sinzuiyes19:50
EdwinGrubbssinzui: r=me. I wonder why we don't have an architecture for optionally queueing up notify() calls and processing them later.20:19
EdwinGrubbsjamalta, leonardr: do you still need a review?20:20
sinzuiEdwinGrubbs: you mean "What happened to rabbitMQ"? I agree this is needed. This was a very rude surprise for me20:20
leonardredwingrubbs: yes, please20:20
leonardrhttps://code.edge.launchpad.net/~leonardr/lazr.restful/generate-multiversion-collections/+merge/1815320:20
EdwinGrubbssinzui: well, these are all internal messages, so they could easily be queued up in a db table by pickling the objects involved.20:21
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin, allenap || reviewing: leonardr, al-maisan<http://tinyurl.com/yag7j48> || queue [jamalta] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
jamaltaEdwinGrubbs: no allenap reviewed the branch, i believe it has to be landed still, though.20:25
jamaltai think allenap ran short on time, i'm not sure where that is at this point20:25
EdwinGrubbsjamalta: since there was just a rollout, non-critical branches probably won't be able to be landed until Monday.20:26
jamaltaEdwinGrubbs: sounds good to me, thanks20:26
EdwinGrubbsjamalta: I can do it for you, but you should probably ping me on Monday so I don't forget.20:27
jamaltaEdwinGrubbs: will jot that down on my calendar, thanks :)20:27
jamaltaand thanks for the help earlier20:27
jamaltait's fun to learn so much from such trivial bugs :)20:27
sinzuiEdwinGrubbs: correct: Our struggle is that since we do not have that infrastucture, I need to proposed schema changes, land model changes, and isolate these on staging. Regardless of the labour, this this a 5 week fix and few users will see it for me to get good feedback :/20:29
sinzuiEdwinGrubbs: Can I resubmit my proposal against the correct branch? for an RC I should make it clear that I want to merge into db-devel20:39
sinzuiEdwinGrubbs: and the diff will be very small20:40
EdwinGrubbssinzui: yes.20:41
=== salgado is now known as salgado-afk
sinzuiEdwinGrubbs: https://code.edge.launchpad.net/~sinzui/launchpad/release-timeout-bug-513321/+merge/1816020:43
=== matsubara is now known as matsubara-afk
EdwinGrubbssinzui: done20:43
sidneian, there we go20:53
sidneianyone up for a lazr-js review?20:54
intellectronicasidnei: sure, i'm happy to take it21:10
sidneiintellectronica, awesome. it's pretty trivial actually. let me find the url21:10
sidneiintellectronica, https://code.edge.launchpad.net/~sidnei/lazr-js/module-config-uses-min21:12
intellectronicasidnei: i'd start the tuple on line 20 of the diff on the next line. your call but i find this a bit weird to read21:14
sidneiintellectronica, good catch.21:15
intellectronicasidnei: no other comment. patch looks great21:15
sidneiintellectronica, cool. is lazr-js still using pqm?21:15
intellectronicasidnei: i think so, yes21:16
sidneiintellectronica, ok. i have to find the majik incantation again *wink*21:16
intellectronicasidnei: and confusingly the trunk branch is called 'toolchain'21:17
intellectronicasidnei: also, i think you'll have to wait until launchpad's pqm is open again. it's still in release-critical mode after the release21:18
sidneiintellectronica, man, that really shouldn't block lazr-js21:18
* sidnei holds up on the angrymail21:18
sidneiintellectronica, is this what you mean? https://pastebin.canonical.com/27105/21:19
intellectronicai totally agree. time is high for us to start including lazr-js as a separate package and test it independently21:19
intellectronicasidnei: yes, exactly21:19
intellectronicamakes it a bit clearer on first pass that you are concatenating tuples21:20
sidneiyeah. it wasn't intentional. i refactored that line a couple times.21:20
EdwinGrubbsleonardr: I got errors in multiversion.txt and field.txt tests.21:49
leonardredwingrubbs, can you paste the errors?21:52
EdwinGrubbsleonardr: http://pastebin.ubuntu.com/364149/21:56
leonardredwin: i think you have a different version of something21:57
leonardredwingrubbs: yes, i believe you have an old version of simplejson21:58
leonardredwingrubbs: i have 2.0.9. what do you have?21:58
EdwinGrubbsleonardr: you're right, I have an old version.22:02
EdwinGrubbsleonardr: do you get an oops when you reload the mp? https://code.edge.launchpad.net/~leonardr/lazr.restful/generate-multiversion-collections/+merge/1815323:21

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!