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

thumpernoodles775: https://code.edge.launchpad.net/~michael.nelson/+activereviews01:43
rockstarthumper, https://code.edge.launchpad.net/~rockstar/launchpad/update-review-table-on-comment/+merge/1743903:17
thumperrockstar: https://code.edge.launchpad.net/~thumper/launchpad/js-play/+merge/1719303:49
rockstarmwhudson, https://code.edge.launchpad.net/~rockstar/launchpad/pedantry-round1/+merge/1744104:28
mwhudson+1 with added swearing04:29
thumpernoodles775: https://code.edge.launchpad.net/~thumper/launchpad/js-play/+merge/1719304:35
stubhttps://code.launchpad.net/~stub/launchpad/bug-504291-disconnection-errors/+merge/17371 is a fix for the High priority Bug #50429106:28
mupBug #504291: DisconnectionErrors (already disconnected) happening again <oops> <Launchpad Foundations:In Progress by stub> <Storm:Invalid> <https://launchpad.net/bugs/504291>06:28
henningeGood morning!07:29
henningeWhen ever a reviewer comes around, I'd be glad to get a review for my branch ... ;-)07:29
=== henninge changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
henningenoodles775: https://code.edge.launchpad.net/~henninge/launchpad/buildmaster-failure/+merge/1744909:59
noodles775Looking09:59
noodles775henninge, done. Thanks for that!10:06
henningenoodles775: I saw it, thanks. Already in pqm.10:07
salgadohenninge, how about exchanging some reviews? ;)11:05
henningesalgado: with pleasure!11:06
salgadohenninge, where's yours?  mine's at https://code.edge.launchpad.net/~salgado/lazr.restful/bug-507447/+merge/1739511:07
henningesalgado: https://code.edge.launchpad.net/~henninge/launchpad/bug-488765-oops-translations/+merge/1739611:07
henningesalgado: done11:12
=== matsubara-afk is now known as matsubara
salgadohenninge, I assume danilo was ok with fixing the bug by changing the view?  I ask because I see no comments from him on the bug after you listed the different ways we could fix it11:33
henningesalgado: danilo is out of order atm ... ;)11:34
henningesalgado: yes, I am sure he is ok with that.11:34
henningesalgado: bug 507534 is the real bug that still needs to be fixed.11:36
mupBug #507534: Product.primary_translatable also lists source packages <Launchpad Translations:New> <https://launchpad.net/bugs/507534>11:36
henningeand that is what danilo propsed, too.11:36
henningeThis is mainly to stop the oops from happening and not getting an empty page.11:37
salgadook11:39
=== salgado changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado changed the topic of #launchpad-reviews to: on-call: salgado || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
bacsalgado: can i get a review for a tiny branch?14:21
salgadobac, sure!14:21
bacsalgado: cool:  https://code.edge.launchpad.net/~bac/launchpad/bug-491320/+merge/1745814:21
bacsalgado: rebooting but i'll be right back14:22
=== henninge_ is now known as henninge
henningesalgado, bac: can you tell me what this TALES statement does exactly?14:26
henningecontext/required:launchpad.TranslationsAdmin14:26
henningeDoes it mean "true if the current user has this permission on the current view"?14:26
salgadohenninge, returns True if the logged in user has the given permission on context14:26
henningecontext, not view, right.14:27
henningesalgado: thanks14:27
salgadobac, what was causing the newlines and spaces to be added to the text area?14:41
bacsalgado, the spaces and newlines between <textarea> and </textarea>14:44
salgadooh, of course14:44
=== salgado is now known as salgado-lunch
=== henninge changed the topic of #launchpad-reviews to: on-call: salgado || reviewing: - || queue [henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
henningesalgado-lunch: I have replied to your review and put another branch up for review. If you could be so kind ... ;)14:50
henningesalgado-lunch: oh, and please don't forget to enter "code" as the review type to keep "ec2 land" happy.14:53
EdwinGrubbsallenap: did you already start reviewing my branch? I was going to ask salgado to review it otherwise.14:58
allenapEdwinGrubbs: I am so sorry, no I haven't. I've been working on this sort-of-critical checkwatches issue and I forgot. Please ask salgado as I'm still working on it.14:59
* allenap tasers himself in an act of modern auto-flagellation for forgetting.15:00
EdwinGrubbssalgado-lunch: can you review this branch. About half of it is simple search and replace on zcml files, so it is actually much easier than the size of the diff would indicate.  https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-451208-subscribing-reveals-email/+merge/1726115:02
bacsalgado-lunch, thanks for the review15:20
=== matsubara is now known as matsubara-lunch
=== salgado-lunch is now known as salgado
salgadoEdwinGrubbs, sure, I'll get to it soon16:04
henningeHi salgado, I will not be around for much longer. Do you think you can have a look at my second proposal?16:04
salgadohenninge, do you know how to enter the review type using the email interface?16:04
salgadodoing that now16:04
henningeyes, just append16:04
henningereview approve code16:04
salgadocool16:04
henningesalgado: thanks a lot!16:05
=== henninge is now known as henninge-brb
=== salgado changed the topic of #launchpad-reviews to: on-call: salgado || reviewing: Edwin || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== henninge-brb is now known as henninge
=== matsubara-lunch is now known as matsubara
=== henninge changed the topic of #launchpad-reviews to: on-call: salgado || reviewing: Edwin || queue [henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
henningesalgado: thanks for the review. There is another branch of mine, just a short one.16:41
henningesalgado: https://code.edge.launchpad.net/~henninge/launchpad/bug-507498-oops-subset/+merge/1745916:41
salgadohenninge, ok16:41
henningesalgado: I will leave now but if it's easy, please just do.16:42
henningesalgado: thank you very much!16:42
salgadowell, you should tell me if it's easy or not. ;)16:42
salgadogary_poster, can we check that an object is security proxied in a way that's less ugly than http://paste.ubuntu.com/357145/?17:01
henningesalgado: I meant, if you don't have any questions ... ;)17:06
henningesalgado: I am off, ttyl ;-)17:08
gary_postersalgado: yes I think so (sorry was on call17:22
gary_postergetting ref17:22
gary_postersalgado: would queryProxy from zope.security work for you (see http://svn.zope.org/zope.proxy/trunk/src/zope/proxy/interfaces.py?rev=75161&view=auto )17:24
salgadogary_poster, I think it should work, let me check with EdwinGrubbs 17:26
salgadoEdwinGrubbs, ^17:26
EdwinGrubbssalgado: I can try that.17:29
=== beuno is now known as beuno-lunch
=== beuno-lunch is now known as beuno
=== deryck is now known as deryck[lunch]
=== deryck[lunch] is now known as deryck
EdwinGrubbssalgado, gary_poster: I'm getting this error: ComponentLookupError: (<InterfaceClass zope.proxy.interfaces.IProxyIntrospection>, '')19:10
gary_posterEdwinGrubbs: it's not a utility19:11
gary_posterEdwinGrubbs: just import the desired function from zope.security19:11
gary_posterEdwinGrubbs: the interface describes what the package offers19:11
gary_posterzope.proxy I mean19:11
gary_posterEdwinGrubbs: ^^^19:12
EdwinGrubbsgary_poster: thanks, that worked19:18
gary_posterEdwinGrubbs: cool, np19:18
EdwinGrubbssalgado: did you want get_naked_vocab() defined just in that one doc test or added to the globals in c/l/testing/systemdocs.py?19:19
salgadoEdwinGrubbs, just that one would be fine, I think19:20
EdwinGrubbssalgado: reply sent. I'm going to lunch now.19:37
=== EdwinGrubbs is now known as Edwin-lunch
salgadoEdwin-lunch, why do you need the "vocab.__module__[:5] != 'zope.'" in your test?19:41
=== salgado changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado is now known as salgado-afk
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== Edwin-lunch is now known as EdwinGrubbs
EdwinGrubbssalgado-afk: because there are a few vocabs that zope registers but not as a secured utility.21:25
EdwinGrubbssinzui: did you want to review my one line fix for deactivating an account or wait until we hear back on how we can add real testing for this?21:49
sinzuiEdwinGrubbs: I would like to review it. I'll look at the history of the problem if I think we need to do something else21:50
EdwinGrubbssinzui: here is the diff. https://pastebin.canonical.com/26685/21:51
EdwinGrubbssinzui: do you want an MP?21:52
sinzuiNo, I'll just look at blame to see some context21:52
EdwinGrubbssinzui: You can make it lp.registry.tests.test_person.TestPerson.test_deactivateAccount_copes_with_names_already_in_use fail in the same way the oops does just by making this change on launchpad_ftest_template.   REVOKE UPDATE, INSERT ON EmailAddress FROM write;21:55
sinzuiEdwinGrubbs: r=me. Every instance of this use was added 11 months ago as a part of the DB reconfiguration. There are no tests for this kind if issue21:59
jamaltaHi, I'm looking into fixing a trivial bug to see if I can get into helping with Launchpad a bit again22:29
jamaltaI was looking at bug #253525 and can only find one place that changesfile is still listed22:29
mupBug #253525: changesfile isn't a word <ppa> <trivial> <ui> <Soyuz:Triaged> <https://launchpad.net/bugs/253525>22:29
jamaltaAnyways, my question is, what would be the best name for it to replace changesfile? The +archive/ppa page now reads "sources.changes"22:30
jamaltahttps://edge.launchpad.net/~schooltool-owners/+archive/ppa/+packages <-- page that still shows changesfile22:30
sinzuijamalta: "sources.changes" will do fine22:31
jamaltasinzui: thanks :)22:31
sinzuijamalta: I think you need to update some stories that verify the text22:33
sinzuilp/soyuz/stories/soyuz/xx-distribution-sources.txt for example22:33
jamaltasinzui: alright, i will make sure to udpate that as well22:34
jamaltaalso, i found a template that references changesfile but can't figure out where that template is loaded22:34
jamaltalp/soyuz/templates//distroseriessourcepackagerelease-index.pt22:35
sinzuijamalta: search for the file name in soyuze/browser/configure.zcml22:35
jamaltasinzui: ah awesome, thanks22:35
sinzuijamalta: configure is not a nice file to visit. That is were the view and template are registered under a name (+something)22:36
sinzuijamalta: This looks like the tests that need updating to pass with your changes. I am not certain though: http://pastebin.ubuntu.com/357284/22:38
jamaltasinzui: right, makes sense.. i actually was trying to find such file to figure out where the template was used :)22:38
sinzuijamalta: `./bin/test -vvc -t soyuz.*stories` will run all the stories to verify you text change.22:41
jamaltait's a bit heavy to take in, though, you're right22:41
=== mup_ is now known as mup
bacsinzui: if you get bored between now and tuesday...  https://code.edge.launchpad.net/~bac/launchpad/bug-499351-batching-dls/+merge/1749222:41
sinzuiI'll review your branch now22:45
sinzuibac: why do you cast the list to a list on line 87 of the diff?22:52
sinzui     list(series_and_releases22:52
bacsinzui: an abundance of caution?22:54
bacstupidity?22:54
bacleftover from when s_r used to be something else, i think.22:54
bacyeah, i dumbly made it a set to start with.  that was suboptimal for many reasons.22:54
sinzuiokay.22:54
sinzuianyway, r=me I think you can drop the cast22:55
bacsinzui: thanks22:56

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