#launchpad-reviews 2009-11-02 * al-maisan changed the topic of #launchpad-reviews to: on call: - || reviewing: -,- || queue: [sinzui, al-maisan<http://tinyurl.com/ycqtx3t>] || This channel is logged: http://irclogs.ubuntu.com/ <al-maisan> intellectronica and henninge: when you open up shop please note that the branch in the queue fixes a release-critical bug. <al-maisan> henninge: when you open up shop please note that the branch in the queue fixes a release-critical bug <henninge> al-maisan: oh right, I am OCR today! ;) * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: -,- || queue: [sinzui, al-maisan<http://tinyurl.com/ycqtx3t>] || This channel is logged: http://irclogs.ubuntu.com/ <al-maisan> henninge: :) <henninge> Shop's open. Please feel free to stroll around and look at the offerings, I'll be with you soon. <henninge> noodles775: I just requested an r-c review from you. Please feel free to ask me about it when you get to it. <noodles775> henninge: will do. * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: al-maisan || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ <henninge> al-maisan: The branch looks really good but I am wondering about test coverage. <al-maisan> henninge: aha <al-maisan> henninge: do you think it needs more/different testing? <henninge> al-maisan: I am just wondering if it would be possible to extend the test to test the case where the user has permission to upload to a series other than currentseries. <al-maisan> henninge: I can do that, no johncrosby@example.net. <henninge> al-maisan: currently it only tests "allowed for currentseries, denied for others" <henninge> al-maisan: cool, I'd feel much better about that. <al-maisan> henninge: I'll ping you with the incremental diff in a few minutes. <henninge> al-maisan: did you already run the test suite on this? I am surprised that no other tests need to be adapted. <al-maisan> henninge: package sets are a pretty narrow niche .. but I will run the package upload tests as well .. thanks for pointing this out! <henninge> al-maisan: np <henninge> al-maisan: one last issue. I am not familiar with _schema_circular_imports.py but I think I see what it does. <al-maisan> henninge: it's the avoidance of circular imports.. <henninge> al-maisan: yes, so I gathered. I am just trying to figure out how it works. <henninge> al-maisan: I trust, though, that this is correct the way you did it, so no real issue for the review. <henninge> ;-) <al-maisan> the LP API metadata is manipulated afterwards out of the interface declaration scope <al-maisan> henninge: thanks <henninge> al-maisan: np, r=me <henninge> (what I meant) <al-maisan> henninge: thank you very much indeed. * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || (r-c requests skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ <stub> henninge: https://code.edge.launchpad.net/~stub/launchpad/replication/+merge/14155 is an rc canditate * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: stub || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ <al-maisan> hello henninge, I needed to tidy up a few things to get the requested tests to work: http://pastebin.ubuntu.com/307456/ <al-maisan> henninge: just want to make sure you approve of these changes as well <henninge> al-maisan: I understand <al-maisan> thanks <henninge> stub: I am onto yours next. <al-maisan> henninge: I also ran all package upload and archive related tests to make sure nothing breaks. <henninge> cool <henninge> al-maisan: so you had to make these changes just to be able to extend the test in this way? <al-maisan> henninge: yes, in order to manipulate archive permissions based on package sets, I needed to change the API to pass the package sets as opposed to their names. <al-maisan> that was the main change really. <henninge> al-maisan: btw, you seem to be using the API exclusively to test this. Is there a reason for that? <al-maisan> henninge: yes, this functionality is presently only exposed via the LP API. <henninge> al-maisan: ah <al-maisan> the UI is still to come <henninge> al-maisan: and now a packageset can only be passed in as an object, not represented by its name, as used to be the case, right? <al-maisan> henninge: yes, the reason being that package names are *not* globally unique any more <al-maisan> now, the combination of a package set name and a distro series is unique <al-maisan> s/package names/package set names/ <henninge> al-maisan: ah, I understand. <al-maisan> I could have added a 'distroseries' parameter but I thought it was more straight-forward to just pass in the package set *object* <henninge> al-maisan: but shouldn't there be some obsolete code now, that did the lookup from name to object? <al-maisan> henninge: let me check quickly <al-maisan> henninge: good catch, the ArchivePermission._nameToPackageset() method is now superfluous. <al-maisan> henninge: http://pastebin.ubuntu.com/307472/ <al-maisan> henninge: actually .. on a second thought .. scratch that .. the methods involved are internal (as in not exposed via the LP API) and it may still be OK to pass in package sets or package set names in this case. <al-maisan> the methods involved = the IArchivePermission methods in the most recent diff <al-maisan> henninge: hrm .. this is a bit complicated .. I guess one important aspect is that IArchivePermission methods are exposed *indirectly* via the IArchive LP API and that's where some of the confusion arose <henninge> al-maisan: sorry, team call atm. Won't be long. <al-maisan> henninge: cool. <henninge> al-maisan: so, your patch remains as it was, right? <al-maisan> henninge: yep <al-maisan> henninge: this change is still required: http://pastebin.ubuntu.com/307456/ <henninge> al-maisan: yeah, I believe you ... ;-) <henninge> al-maisan: and the new test looks very good! Thanks for adding that! <henninge> al-maisan: still r=me <al-maisan> henninge: thank you for prodding me :) <henninge> ;) <al-maisan> .. and thanks for the review <henninge> np * henninge lunches <al-maisan> hello henninge-lunch, I have another release critical branch that needs a review: https://code.edge.launchpad.net/~al-maisan/launchpad/sync-470411/+merge/14301 <al-maisan> henninge-lunch: believe it or not but it's a 1-liner :) <henninge> al-maisan: done, r=me <al-maisan> henninge: thanks a million :) <henninge> al-maisan: nee, war doch nur *eine* Zeile! ;-) <al-maisan> aha :) <henninge> al-maisan: eine Million Zeilen hÃ¤tte ich abgelehnt ... <al-maisan> henninge: ich hÃ¤tte es Dir nicht 'mal Ã¼belgenommen :) * abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: stub || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ * abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: stub, - || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ <wgrant> jml: Can you please review https://code.edge.launchpad.net/~wgrant/launchpad/distroseries-source-format-selection-db/+merge/14304? * danilos changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: stub, - || (r-c candidates skip the queue) queue: [sinzui, danilo, danilo, danilo] || This channel is logged: http://irclogs.ubuntu.com/ <danilos> henninge, abentley: mine are not RC-candidates, so do give precedence to what is :) * henninge changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: -, - || (r-c candidates skip the queue) queue: [sinzui, danilo, danilo, danilo] || This channel is logged: http://irclogs.ubuntu.com/ <henninge> danilos: I don't see why strip_last_newline is needed? <danilos> henninge, because gazillion of tests would have to be changed if it was something like rstrip("\r\n") <henninge> danilos: ;-) <danilos> henninge, well, not gazillion, but something like 4 very obscure tests, and if it was just .strip(), then something around 14 :) <henninge> danilos: ok, but my hunch was correct, that rstrip would be enough to do what you need! <danilos> henninge, also, .rstrip() strips all final newlines, and with some comments, whitespace should stay there <danilos> henninge, almost, I started with that, but tests correctly brought header comments to my attention, and they usually have a few newlines before the initial message, and I feel they belong there :) <danilos> henninge, I would have changed the few tests if it were not for these cases where we should keep extra newlines <henninge> danilos: sorry, phone call <danilos> henninge, was it for me? <henninge> danilos: ;-) <henninge> danilos: no, it was my dad <henninge> danilos: the other thing: strip_last_newline is testing for all versions of line breaks, but you only do split("\n") eventually. <danilos> henninge, oh, ok then :) <henninge> danilos: is there a magic in there that I forget that does line break type detection? <danilos> henninge, well, I don't have to fix all problems with that code? also, .split("\n") would work correctly with DOS-style line endings as well <danilos> henninge, and no, there isn't anything special that you are missing <henninge> >>> "Henning\r\nEggers".split("\n") <henninge> ['Henning\r', 'Eggers'] <danilos> henninge, see :) <henninge> danilos: if you are fine with that behaviour ... <danilos> henninge, now, you can insert "#" in front of 'Henning\r' and 'Eggers' and it'll be fine <danilos> henninge, we've got another set of bugs for dealing with \r's, but I am not touching those right now :) <henninge> danilos: ok, ok. r=me * henninge changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || (r-c candidates skip the queue) queue: [sinzui, danilo, danilo] || This channel is logged: http://irclogs.ubuntu.com/ <danilos> henninge, thanks * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com/ <sinzui> abentley: I have an rc-candidate to fix a problem with the product-release-finder: https://code.edge.launchpad.net/~sinzui/launchpad/prf-files/+merge/14311 <sinzui> oh, that is a bad diff * sinzui deletes and tries again <sinzui> abentley: RC candidate: https://code.edge.launchpad.net/~sinzui/launchpad/prf-files/+merge/14312 <abentley> sinzui: r=me. <sinzui> thanks <abentley> sinzui: You should make two separate review requests, one for RC one for code. <sinzui> yes, you are right <sinzui> I added RC so that you would see it should hop in front of non-rc branches <abentley> sinzui: If you requested an RC review from flacoste, I would see that also. <sinzui> I see that I was not thinking <abentley> sinzui: Since I can't provide an "rc code" review, the Canonical Launchpad Engineering review request will never be satisfied. <flacoste> sinzui: do you have a r-c review request for me? <abentley> flacoste: We were discussing https://code.edge.launchpad.net/~sinzui/launchpad/prf-files/+merge/14312 * abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com/ #launchpad-reviews 2009-11-03 <noodles775> Hi stub, is your branch (bug 461800) already through ec2 test etc., and ready to land? (or is there other manual testing that you had to do?) <mup> Bug #461800: new-slave.py no longer works <current-rollout-blocker> <Launchpad Foundations:In Progress by stub> <https://launchpad.net/bugs/461800> <stub> noodles775: That script is only tested on staging - everything in database/replication is outside of our test suite. <noodles775> stub: great, so land away then :) <stub> noodles775: ok. It was a branch of db-devel - is that going to be a problem with the current buildbot situation? <noodles775> stub: strange? the MP says that it's for merging into devel... * stub double checks <stub> noodles775: Yes, you are right. It can land on launchpad/devel. <noodles775> stub: would it be quicker to get it qa'd if you land directly into db-devel? <noodles775> I'm about to try sending off a manual ec2 test run of db-devel... <stub> noodles775: Not really <noodles775> OK, then devel it is. Thanks! <al-maisan> hello gmb, when you start your day: I have a branch fixing an r-c bug for review: https://code.edge.launchpad.net/~al-maisan/launchpad/subsets/+merge/14343 * al-maisan changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [al-maisan] || This channel is logged: http://irclogs.ubuntu.com/ * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || (r-c candidates skip the queue) queue: [al-maisan] || This channel is logged: http://irclogs.ubuntu.com <gmb> al-maisan: Thanks. I'll get started on it shortly. <al-maisan> gmb: thanks * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: al-maisan || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> al-maisan: Looks good. r=me. Go ahead and request an rc review for it. <al-maisan> gmb: thank you very much. * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <al-maisan> noodles775: ^^ <noodles775> Thanks al-maisan * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: lunch || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <al-maisan> gmb: here's another likely r-c candidate: https://code.edge.launchpad.net/~al-maisan/launchpad/getbyname-472608/+merge/14354 <al-maisan> please take a look when you get back. * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: al-maisan || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> al-maisan: Okay, looking now. <al-maisan> gmb: thanks! <gmb> al-maisan: Looks good, r=me. <al-maisan> gmb: thank you very much! <gmb> al-maisan: You know it's a good review day when you get small Soyuz branches ;) <al-maisan> gmb: :) <bac> gmb: quiet day? i'll be joining you here after i grab a quick bite to eat. <gmb> bac: Very quiet. I've reviewed entirely 2 RC requests, so everything must be working perfectly and we need fix nothing ever again. <bac> gmb: your assessment is spot on IMO. perhaps i'll take a longer breakfast in light of our accomplishments. <gmb> A wise move. <noodles775> hey bac! There's 3 'In progress' registry crb bugs listed at https://bugs.edge.launchpad.net/launchpad-project/+bugs?field.searchtext=&orderby=-importance&assignee_option=any&field.assignee=&field.bug_reporter=&field.bug_supervisor=&field.bug_commenter=&field.subscriber=&field.omit_dupes.used=&field.has_patch.used=&field.has_cve.used=&field.tag=current-rollout-blocker&field.tags_combinator=ANY&search=Search <noodles775> woops, sorry. <noodles775> But just wondering whether you know if they are really still in progress, or what their status is? * bac looks <bac> noodles775: i know the two distroseries timeouts are being investigated. i'm not sure about the other. <bac> noodles775: based on the last comment of bug 437184 i'd guess that edwin is working on it <mup> Bug #437184: New "See full publishing history" location is less useful <current-rollout-blocker> <post-3-ui-cleanup> <trivial> <Launchpad Registry:In Progress by edwin-grubbs> <https://launchpad.net/bugs/437184> <noodles775> bac: thanks - it's just getting quite late if they need QA on staging. <bac> noodles775: yes, i agree <leonardr> gmb, can you put https://code.edge.launchpad.net/~leonardr/launchpadlib/choose-your-client/+merge/14361 in the queue? <gmb> leonardr: Sure * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: leonardr || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> leonardr: RC candidate? <leonardr> gmb: no * bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: leonardr, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> bac: Could you take a look at https://code.edge.launchpad.net/~gmb/launchpad/affects-me-spinner-bug-458766/+merge/14360 for me? It's not RC. * bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: leonardr, gmb || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> bac: I've just noticed that I've introduced some lint in the last hunk of the diff. Please ignore that; I'm fixing it now. <bac> ok <gmb> leonardr: Your branch looks good. r=me. <leonardr> gmb, that was quick, ok <gmb> leonardr: No, I'm just single-minded. I can review it again if you'd like ;) <leonardr> haha <gmb> I've also had two years of reviewing cprov's Soyuz patches. That makes you faster at reading code... * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: -, gmb || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, gmb || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> argh <al-maisan> :) * Ursinha changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, gmb || (r-c candidates skip the queue) queue: [Ursinha] || This channel is logged: http://irclogs.ubuntu.com <gmb> Ursinha: Do you have an m-p for your branch? * gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: Ursinha, gmb || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <Ursinha> gmb, yes sir <Ursinha> gmb, https://code.edge.launchpad.net/~ursinha/launchpad/bug-391569-add-last-changed-column/+merge/14366 <gmb> Ursinha: Thanks. <gmb> Ursinha: Is this RC? <bac> gmb: r=bac. sorry it took so long, i got pulled into a long stand-up call * bac changed the topic of #launchpad-reviews to: on call: gmb, - || reviewing: Ursinha, gmb || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> bac: No worries, it's not RC. <gmb> Thanks. <bac> gmb: it even works with webkit <gmb> bac: Glad you said so... I was wondering if it might just be for me that it worked. Webkit compatibility seems more'n a bit flaky in places. <bac> gmb: yeah, that's why i try testing stuff where possible * bac changed the topic of #launchpad-reviews to: on call: gmb, bac|| reviewing: Ursinha, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> Ursinha: r=me with some stylistic changes (see my comments, they're pretty minimal). <gmb> Nice job. * gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: -, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> bac: I'm going to go an try and get some hacking done. Give me a shout if the sky starts falling. <bac> gmb: ok. have a call at the top of the hour that may last an hour or so. can you keep on eye out here during that time? <gmb> Sure. <bac> gmb: thanks <adeuring> bac: could you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-435556/+merge/14368 ? (140 lines diff) <bac> adeuring: is this for an RC? <adeuring> bac: no <bac> adeuring: i have a call starting in 4 minutes. i can review it or in an hour or you can ask gmb <adeuring> bac: sure, no problem. (i think gmb doesn't want to do more reviews today) <bac> adeuring: ok i'll get to it when i can * bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || (r-c candidates skip the queue) queue: [abel] || This channel is logged: http://irclogs.ubuntu.com <adeuring> bac: sure, no hurry. (the branch will sit anyway around until PQM opens again) <gmb> bac, abel: I'll take it - I was OTP. * gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: abel, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> adeuring: ^^ * gmb curses nickname changes across servers <adeuring> gmb: thanks! * bac changed the topic of #launchpad-reviews to: on call: bac-otp || reviewing: abel, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * gmb changed the topic of #launchpad-reviews to: on call: gmb, bac-otp || reviewing: abel, - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <Ursinha> gmb, thanks for your review and the tips <gmb> adeuring: Can you re-push your branch? There's no diff showing up and I can't branch from it; I get a "not a branch" error. <gmb> Ursinha: You're welcome. <adeuring> gmb: done. new branch, new MP <gmb> adeuring: Thanks. * gmb goes to get a drink while the branch updates <gmb> adeuring: Weird; that branch still shows as "Not pushed to yet" Could you push it to another location on LP for me? I wonder if something in codehosting has gone *boing* <adeuring> gmb: what do you mean with "other location"? <gmb> adeuring: Try `bzr push lp:~adeuring/launchpad/foo` rather than just `bzr push`. <adeuring> gmb: ah, ok <adeuring> gmb: lp:~adeuring/launchpad/review-435556 * gmb looks <gmb> adeuring: Yep, I think something on codehosting is broken; both branches are showing as not having been pushed to. <adeuring> gmb: right, but you can branch from them. <gmb> adeuring: Interesting; I can't. <adeuring> odd... <gmb> Ah, well, I can't *merge* from them. Hang on... <adeuring> gmb: here's the diff: http://paste.ubuntu.com/308634/ <gmb> adeuring: Thanks. <gmb> adeuring: Your changes look good. r=me. <adeuring> gmb: thanks1 <adeuring> erm, thanks! * gmb changed the topic of #launchpad-reviews to: on call: bac-otp || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <adeuring> gmb: could you please formally give your review here: https://code.edge.launchpad.net/~adeuring/launchpad/bug-435556/+merge/14370 ? * bac changed the topic of #launchpad-reviews to: on call: bac-lunch || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <gmb> adeuring: Done <adeuring> gmb: thanks! <EdwinGrubbs> bac: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/disable-summaries-for-distroseries/+merge/14374 <sinzui> EdwinGrubbs: thanks for getting this done so quickly <EdwinGrubbs> sinzui: bac doesn't seem to be around, can you review ^^^ <sinzui> I will <sinzui> Edwin: I am not excited by the "Not displayed on distroseries page for performance reasons." <sinzui> Also, this is both distroseries +index and +milestone <sinzui> The change also removes the milestone summary <sinzui> EdwinGrubbs: You could replace the technical message with <sinzui> <tal:summary content="structure milestone/summary/fmt:shorten/80" /> <sinzui> so that users can see something about the milestone <EdwinGrubbs> sinzui: are you saying that you don't want this change to affect both the distroseries +index and +milestone? <EdwinGrubbs> sinzui: oh, I see, you are saying that the comment about the "distroseries page" is inaccurate. <sinzui> correct. <sinzui> Your fix should fix both page :) * bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <EdwinGrubbs> sinzui: new diff posted: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/disable-summaries-for-distroseries/+merge/14374 <sinzui> EdwinGrubbs: Looks good to land. <EdwinGrubbs> sinzui: thanks <EdwinGrubbs> noodles775, flacoste: can I get an RC for https://code.edge.launchpad.net/~edwin-grubbs/launchpad/disable-summaries-for-distroseries/+merge/14374 <al-maisan> hello bac, I have a branch that fixes an r-c bug: https://code.edge.launchpad.net/~al-maisan/launchpad/uploadurl-472929/+merge/14382 <al-maisan> Could you please take a look? <bac> al-maisan: on it <al-maisan> bac: thanking you. <flacoste> EdwinGrubbs: done <EdwinGrubbs> thanks <bac> al-maisan: at line 16 of your diff, is self.context.distribution a dictionary? <al-maisan> bac: self.context is an IArchive instance <al-maisan> and distribution is one of its properties <bac> al-maisan: right but your indexing into it with ...distribution[series] so i wonder if that is a dict <bac> argh! s/your/you're <al-maisan> bac, that's right, Archive implements a __get_item__() method <bac> al-maisan: so can you use .get(series) and avoid the try/NotFoundError ? <al-maisan> bac, wait, it's Distribution that has that method. <al-maisan> bac, let me look. <bac> al-maisan: yes, see if you can collapse lines 15-18 to <bac> series = self.context.distribution.get(series) <al-maisan> bac, ah, I see what you mean. Just treat it like a dict. <bac> yes <al-maisan> hmm .. <al-maisan> File "/home/mhr/canonical/lp-branches/uploadurl-472929/lib/lp/soyuz/browser/archive.py", line 294, in _traverse_permission <al-maisan> series = self.context.distribution.get(series) <al-maisan> ForbiddenAttribute: ('get', <Distribution at 0xbbef1d0>) <al-maisan> bac ^^ <al-maisan> I was trying http://pastebin.ubuntu.com/308740/ <bac> al-maisan: ok then! <al-maisan> sorry <bac> al-maisan: i'm confused by the multiple checks at 13 and 19. can 20-21 be moved under the check at 13? * al-maisan looks <bac> perhaps not <al-maisan> bac: so, on lines 15-18 I am making sure that the series passed in by the user (via an URL param) is actually there. <al-maisan> if I can't find it, it is the same as if it was missing i.e. the result is a 404 <al-maisan> otherwise I look up the package set, and, if I find that I attempt to find at least one archive permission <al-maisan> bac: please note also that I tested the branch on the Soyuz dogfood system. <bac> al-maisan: was there a reason for s/grouchy-thunderbird/thunderbird/ ? <al-maisan> bac: yes, I wanted 2 package sets with the same name <bac> ah, ok <al-maisan> in order to test that they are disambiguated on a distro series basis <bac> gotcha <bac> al-maisan: r=bac. thanks! <al-maisan> bac: thanks a million <al-maisan> !! <bac> al-maisan: you going to try for an r-c from flacoste? <al-maisan> bac: I did ping him on lp-code <al-maisan> flacoste: the branch in question is: https://code.edge.launchpad.net/~al-maisan/launchpad/uploadurl-472929 <al-maisan> could I please have an r-c for it? <al-maisan> flacoste: sorry for nagging but the midnight UTC deadline is drawing closer. <flacoste> al-maisan: has this been QA-ed on dogfood yet? <al-maisan> flacoste: yes indeed. <flacoste> al-maisan: ok, rc=me, did you update LPS with your UPDATE query yet? <al-maisan> flacoste: I am working on it as we speak. <al-maisan> flacoste: thanks for the rc. * bac changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <mwhudson> abentley: want to review this branchmergeproposaljob join fix? <mwhudson> it's about 15 lines <abentley> mwhudson: Okay. <mwhudson> abentley: you should have an email by now <abentley> mwhudson: r=me <mwhudson> abentley: thanks #launchpad-reviews 2009-11-04 * jtv changed the topic of #launchpad-reviews to: on call: jtv || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <jtv> stub: care to review this tiny reordering? https://code.edge.launchpad.net/~jtv/launchpad/message-sharing-rearrange-commits/+merge/14342 <stub> approved <jtv> hi mrevell! I've written up that article about trying out Translations on staging. Care to proof-read it? (Warning: it's pretty long). <mrevell> jtv, I'd love to, thanks man. Wanna email it or do you have a link? <jtv> mrevell: the email is on its way <jtv> mrevell: what does strike me about it is how long it all gets, what with the bzr setup. Wish there were some easy https-based way to get people started or something. It's liberally interspersed with links to your documentation though. <jtv> (14:16:36) jtv: mrevell: the email is on its way <jtv> (14:17:44) jtv: mrevell: what does strike me about it is how long it all gets, what with the bzr setup. Wish there were some easy https-based way to get people started or something. It's liberally interspersed with links to your documentation though. * jtv changed the topic of #launchpad-reviews to: n call: - || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * jtv changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <jtv> stub: removing the entire debug output stanza and gc.collect() from the script for production... that ok with you? <jtv> I mean, in the branch you reviewed <stub> jtv: Yup <jtv> thx * EdwinGrubbs changed the topic of #launchpad-reviews to: on call: EdwinGrubbs || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <sinzui> barry: ping <sinzui> barry: noodles775_: flacoste: I hope for a review and an RC for https://code.edge.launchpad.net/~sinzui/launchpad/full-page-width/+merge/14427 <flacoste> sinzui: why did you add a .full-page-width CSS class? <flacoste> sinzui: ignore me, i understand <flacoste> sinzui: can you add a comment above the CSS rule pointing to a test-case for it (what people should look at when they update it? <sinzui> There is no nest case for it. The behaviour was seen because the markup was invalid <sinzui> flacoste: I considered not adding the CSS rule. The nesting experience as a surprise. It was not something obvious, so I added a rule to prevent it from happening again. It is like the clear rules in the CSS that define after rules to ensure the core rule is consistent <flacoste> sinzui: that in a comment would be enough <sinzui> okay <sinzui> /* The content is already full width; the content should remain full width. */ <sinzui> flacoste: The branch that landed the invalid markup was fixing the layout of two edit icons for webkit. There were no test changes, we just reviewed the UI <al-maisan> Hello EdwinGrubbs, could you please review https://code.edge.launchpad.net/~al-maisan/launchpad/parc-456551/+merge/14430 ? <al-maisan> It's a potential r-c candidate. <EdwinGrubbs> al-maisan: Im out of the office running an errand. Is this something i can review by just looking at the diff on my phone? <al-maisan> EdwinGrubbs: sure, http://pastebin.ubuntu.com/309759/ <al-maisan> EdwinGrubbs: please note: it is the first 20 lines of the diff that matter. <al-maisan> EdwinGrubbs: I need to grab dinner but will be back later. <EdwinGrubbs> al-maisan: What is the last else? <al-maisan> EdwinGrubbs: the last else are primary archive (ubuntu) uploads <al-maisan> else: <al-maisan> archives = [self.policy.archive] <EdwinGrubbs> al-maisan: i dont quite understand the difference between the archive for the elif and the one for the else <al-maisan> EdwinGrubbs: the 'elif' clause is for uploads to PPAs <EdwinGrubbs> al-maisan: r=me <al-maisan> EdwinGrubbs: thanks! <Ursinha> all: do I have to do something else besides filing a merge proposal to get a db review from stub? #launchpad-reviews 2009-11-05 <danilos> EdwinGrubbs, hi, do you have time to take a look at https://code.edge.launchpad.net/~danilo/launchpad/bug-474874/+merge/14450 * danilos changed the topic of #launchpad-reviews to: on call: EdwinGrubbs || reviewing: - || (r-c candidates skip the queue) queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com <danilos> noodles775, al-maisan: I assume you are not doing any reviews today? <al-maisan> danilos: if need be I can review. <danilos> al-maisan, it's very short, just adding some DB permissions for a script: https://code.edge.launchpad.net/~danilo/launchpad/bug-474874/+merge/14450 <noodles775> danilos: I'm not no, sorry. <al-maisan> danilos: OK .. give me 5 minutes and I'll review it. <danilos> al-maisan, excellent, thanks a lot <al-maisan> you are welcome <danilos> noodles775, no problem, al-maisan is taking care of it :) <al-maisan> danilos: this is supposed to go into 'devel' .. is that correct? <danilos> al-maisan, yes, but I am actually going to ask for this to be CPed (or perhaps not, GRANTs might be enough) <danilos> al-maisan, note that security.cfg changes do not need to go into db-devel <al-maisan> danilos: fine, just checking. <al-maisan> danilos: looks good, r=me <danilos> al-maisan, thanks * EdwinGrubbs changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com * danilos changed the topic of #launchpad-reviews to: on call: - || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <adeuring> al-maisan: fancy a relatively small review? (or is already time for "Feierabend" for you?) <al-maisan> adeuring: a small one is OK. <adeuring> al-maisan: thanks! https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-udev-submissions-noise-reduction-2/+merge/14479 (130 lines; just two tuples changed) <sinzui> beuno: ping <beuno> sinzui, hi <sinzui> your suggest for the yes-no widget rocks. it hard to do <al-maisan> adeuring: OK <sinzui> beuno: I'll work on it later today and see if I can make a universal dsolution <beuno> sinzui, because it uses db enums, yadayadayada? <sinzui> beuno: no actually not <sinzui> beuno: The launchpadformview custom_widget function does not work with macro-style widgets <sinzui> beuno: making my widget work or fixing LaunchpadFormView is a big win for reuability <al-maisan> adeuring: so, this is a 'devel' branch, right? <adeuring> al-maisan: yes <al-maisan> cool. <sinzui> beuno: Your suggestion is *much* better that what I did. I spent more than an hour trying to craft a sentence that would work with yes/no. or true/false, or on/off. * sinzui thinks all the boolean radio macros suck. <beuno> sinzui, so I don't think yes/no will work well, no matter how we craft the phrases, if this is an urgent branch, I'd be happy to trade an approve now for a fix before week 4 <sinzui> beuno: no not urgent. I completely agree with your suggest <beuno> sinzui, so can we still be friends if I push back on this? <sinzui> beuno: I think you misunderstand. I have no intention of landing this branch until that widget rocks. <al-maisan> adeuring: LP is still chewing on lp:~adeuring/launchpad/hwdb-udev-submissions-noise-reduction-2 <beuno> sinzui, ah! great! :) <al-maisan> I can't even merge it <adeuring> al-maisan: did you try to simply branch it? <al-maisan> yes <beuno> sinzui, other branch approved, it's a great improvement, thank you <al-maisan> adeuring: this is very funny: <al-maisan> $ bzr merge lp:~adeuring/launchpad/hwdb-udev-submissions-noise-reduction-2 <al-maisan> bzr: ERROR: Not a branch: "bzr+ssh://bazaar.launchpad.net/~adeuring/launchpad/". <adeuring> al-maisan: odd... Let me paste the diff <al-maisan> am I missing something? <al-maisan> adeuring: please do. <adeuring> al-maisan: http://paste.ubuntu.com/310697/ * al-maisan looks <al-maisan> adeuring: r=me <adeuring> al-maisan: thanks! <adeuring> al-maisan: und schÃ¶nen Feierabend! <al-maisan> adeuring: in 30 minutes or so :) but thanks anyway! <gary_poster> barry: I'd like your thoughts (consider it somewhere between a late pre-impl and a pre-review :-) ) on this: http://paste.ubuntu.com/310732/ It is supposed to be a fix for https://bugs.edge.launchpad.net/launchpad-foundations/+bug/475550 <mup> Bug #475550: _pythonpath whitelist of clean_modules is too fragile <Launchpad Foundations:In Progress by gary> <https://launchpad.net/bugs/475550> * barry looks <gary_poster> ty <gary_poster> barry: The old warning still has value, now that I think of it. Here's a variant: <gary_poster> If I find any of the namespace packages using the approach in the pastebin, then I go through sys.modules again. <gary_poster> if anything in sys.modules begins with ``${one_of_the_namespace_packages}.`` (notice the dot) then blow u[ <gary_poster> up <gary_poster> complaining, as before, that _pythonpath should be imported earlier <barry> gary_poster: what's an example of "fewer attributes" (or really different number of them)? <gary_poster> argh, my second idea doesn't work. OK, here's the idea. <gary_poster> but I can make it work. <gary_poster> barry: In this example, ``example`` is a "real" package with only an empty __init__.py <gary_poster> >>> import example <gary_poster> >>> dir(example) <gary_poster> ['__builtins__', '__doc__', '__file__', '__name__', '__path__'] <gary_poster> >>> import lazr <gary_poster> >>> dir(lazr) <gary_poster> ['__doc__', '__name__', '__path__'] <gary_poster> lazr is one of these namespace things <gary_poster> so, barry, I think what I can do instead is look for __file__ <gary_poster> no <gary_poster> that might not match some C modules, right? <barry> right <gary_poster> argh <gary_poster> barry: But C modules would actually have something exposed *other* than just those three, so the approach as given works. What it *can't* do is actually recognize if there's a problem (e.g., lazr.uri was imported before) and complain, unless I switch to just hardcoding the namespace packages. <gary_poster> I'd rather not do that. a different kind of fragility <barry> so inspect.getfile() actually tests for object.__file__ to see if it's a builtin module or not <barry> right <gary_poster> btw, I discovered why our deb zope packages don't have this problem: they are installed in debian with the egg-style __init__.py, not this crazy .pth thing. <barry> maybe... <barry> 1. look for module.__file__; if it exists it's fine <barry> 2. if not, look for module.__package__; if it exists it's a builtin; if not it's a pth namespace package <barry> >>> import _socket <barry> >>> _socket.__package__ <barry> <gary_poster> huh <barry> >>> for name in dir(_socket): <barry> ... if name.startswith('_'): <barry> ... print name <barry> ... <barry> __doc__ <barry> __name__ <barry> __package__ <barry> <gary_poster> barry, cool. That might work. To make sure I understand, I am right that the presence of __package__ in this case is kind of random--that is, an implementation detail that might very well change? <gary_poster> s/I am/am I/ <barry> gary_poster: i'd have to look at the code to know that ;) <gary_poster> heh, ok <barry> gary_poster: oh, let me also check py2.4 <gary_poster> For me on 2.4: <gary_poster> >>> import _socket <gary_poster> >>> _socket.__package__ <gary_poster> Traceback (most recent call last): <gary_poster> File "<stdin>", line 1, in ? <gary_poster> AttributeError: 'module' object has no attribute '__package__' <barry> fsk <gary_poster> otoh, it has a __file__ ;-) <barry> yeah, so "implementation detail that might very well change" == True <gary_poster> right <barry> how strange! why did python 2.6 lose _socket.__file__?! <barry> seems like that would still be pretty handy, and that _socket.__package__ is useless <barry> yay for nothing in the NEWS file about this <sinzui> barry: gary_poster: I have a small RC candidate for review: https://code.edge.launchpad.net/~sinzui/launchpad/noneable-bug-475433/+merge/14487 <barry> sinzui: looking... <barry> sinzui: mp has no diff yet <sinzui> oh bugger <sinzui> I think we are waiting for both the branch scanner and the mp cron <sinzui> barry: do you want a pastebin? <barry> sinzui: sure <sinzui> http://pastebin.ubuntu.com/310757/ <barry> sinzui: r-me <barry> er, r=me <gary_poster> barry: I think this will work. <gary_poster> (1) if there's a __file__ leave it alone; it's fine. <gary_poster> (2) if there is a name in the dir that is not __*__ and the full dotted name is not in sys.modules, leave it alone, it's fine. <gary_poster> (3) Otherwise, we have a namespace package, and we can act on that. Moreover, if there *were* any names in sys.modules, that means that _pythonpath was imported too late. <sinzui> barry: thanks <gary_poster> Going to sketch that <barry> gary_poster: sounds reasonable. i'm going to grab some lunch now. feel free to paste anything for when i get back <gary_poster> barry: thank you, will do. <gary_poster> barry, fwiw: http://paste.ubuntu.com/310788/ <gary_poster> going to make an MP <gary_poster> flacoste, barry: https://code.edge.launchpad.net/~gary/launchpad/pythonpath/+merge/14490 <flacoste> gary_poster: i'll leave barry do the actual code review <gary_poster> flacoste: cool <gary_poster_> barry: I'm on this nick for a sec :-) <sinzui> flacoste: I have a small RC candidate for review: https://code.edge.launchpad.net/~sinzui/launchpad/noneable-bug-475433/+merge/14487 <flacoste> sinzui: approved, add it to CRB <sinzui> thanks. <barry> gary_poster: looking <gary_poster> barry: thanks. on call, so will have latency <barry> gary_poster: r=me <gary_poster> barry, thank you <abentley> rockstar: I can has review? https://code.edge.launchpad.net/~abentley/launchpad/fix-rewrite/+merge/14498 <rockstar> abentley, is this for RC? <abentley> rockstar: Yes. <rockstar> abentley, I wonder if we should also fix the stdout flushing issue <rockstar> abentley, also, r=me <abentley> rockstar: thumper wanted to fix it by invoking the python interpreter with -u, so I've gotten mbarnett to do that. <rockstar> abentley, ah, okay. great. #launchpad-reviews 2009-11-06 <jtv> Urgent review needed, folks! <jtv> It's for a good causeâunbreaking buildbot <jtv> and enabling a re-roll * adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <adeuring> jtv: what do you need reviewed? <jtv> adeuring: https://code.edge.launchpad.net/~jtv/launchpad/db-bug-476218/+merge/14532 <jtv> tiny but urgent :-) * adeuring is looing <jtv> dankeschÃ¶n <adeuring> jtv: r=me <jtv> adeuring: vielen Dank! <adeuring> jtv: gern geschehen :) <jtv> ...und gerne gesehen :-) * noodles775 r-c's <noodles775> jtv: ok, land away :) * jtv approaches the landing strip <gmb> adeuring: Good morning. Would you be able to take a look at https://code.edge.launchpad.net/~gmb/launchpad/subscriber-portlet-timeout-bug-393476/+merge/14502 for me? <adeuring> Morning gmb, sure, I'll look <gmb> Thanks. <adeuring> gmb: r=me <gmb> adeuring: Excellent, thanks <wgrant> adeuring: Thanks. <adeuring> wgrant: welcome <wgrant> adeuring: I have another really easy (sed) one there, if you have time... <adeuring> wgrant: sure <adeuring> wgrant: I am good in forgetting small things. So, don't hestitate to kick me next week, when PQM opens again, to submit you branch <wgrant> adeuring: Heh, will do. Thanks. <wgrant> adeuring: can you flip the MP status itself to Approved? <adeuring> wgrant: done. (good example how i can forget things ;) <wgrant> adeuring: Well, *everybody* forgets that. <adeuring> wgrant: r=me for yoor brnach with the sample data fix <wgrant> adeuring: Thanks muchly. * salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com * barry changed the topic of #launchpad-reviews to: on call: adeuring,salgado,barry || reviewing: -,-,- || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <barry> salgado, adeuring would either of you care to review a fairly simple change to a contrib script in launchpadlib? https://code.edge.launchpad.net/~barry/launchpadlib/475547-cmb/+merge/14482 <salgado> barry, sure, I'll take it <barry> salgado: thanks <salgado> barry, r=me <barry> salgado: thanks! * sinzui changed the topic of #launchpad-reviews to: on call: adeuring,salgado,barry || reviewing: -,-,- || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com <sinzui> adeuring: salgado: barry: I have a small fix https://code.edge.launchpad.net/~sinzui/launchpad/cancel-gpg-bug-391630/+merge/14553 <adeuring> I'd like to duck out... working on an urgent bug... * adeuring changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,- || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com <sinzui> my work is not urgrent <salgado> sinzui, you might want to check test_logintoken.py in lib/c/l/browser/tests. it has some infrastructure for testing the Cancel action of logintoken views <salgado> I think your test could be moved there and benefit of such infrastructure <sinzui> salgado: Does it know then the view is missing with the form so that we never get to the cancel action? <salgado> missing with the form? <sinzui> salgado: The test was pretty conclusive when I added the field that the user submits. I walked through the initialize() method and then validate() to make sure the cancel action was reached <sinzui> salgado: The view's initialize() method makes the field required. The form is always validated before the success action is called <salgado> sinzui, right, but I don't see why you're telling me all that <salgado> I mean, is this going to be a problem if you use LPFormHarness to test the view? <sinzui> salgado: I think the view was at fault, and the test belongs in the view... <sinzui> salgado: I do not see test_logintoken.py <salgado> sinzui, it's on db-devel <sinzui> so is my branch <salgado> and test_logintoken.py is a view test <salgado> lib/canonical/launchpad/browser/tests/test_logintoken.py <sinzui> bugger, I think I branched from the wrong tree <sinzui> i did * sinzui adds test <sinzui> salgado: the test harness is bad <sinzui> I added the test, then removed the change I made to the initialize() method. The test passed. <sinzui> Oh, I see, the test is indeed naive on the very point is was trying to make. salgado, The test assumes that no data is submitted during the cancel action. That is wrong. <salgado> sinzui, does it assume that or does it just don't expect anything to be submitted? <salgado> s/anything/any data/ <sinzui> The form data is hardcodes to an empty dict. I have a fix <sinzui> I do not have a fix * sinzui looks again <salgado> sinzui, don't bother with that if it's too much work. I only suggested it because I thought it could make things simpler <sinzui> I am bothered and I am going to make it work. <EdwinGrubbs> salgado, barry: I have to go to lunch now, but I have a pretty easy branch for review at https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-234051-bug-project-info/+merge/14556 <barry> Edwin-lunch: i can do it * barry changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,edwin || (r-c candidates skip the queue) queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com <sinzui> salgado: I will retrack my branch and set the bug as fix-released by you. Your validator changes were a global fix <salgado> no way <salgado> all of that just because you branched from the wrong tree. :/ <sinzui> yep * sinzui finds lunch * sinzui changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,edwin || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <beuno> sinzui, I will wait for your review on Edwin's branch before I say anything <barry> beuno: on edwin's branch, i like that he's added the pillars under the bugs, but why aren't they all linked? <beuno> barry, good comment to make on the mp ;) <barry> beuno: exactly :) <barry> beuno, Edwin-lunch branch reviewed * barry changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,- || (r-c candidates skip the queue) queue: [] || This channel is logged: http://irclogs.ubuntu.com <sinzui> beuno: Edwin-lunch: I just commented about the alignment of the badges * sinzui changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com <sinzui> barry: can you take a brief look at https://code.edge.launchpad.net/~sinzui/launchpad/latest-release-portlet-bug-424893/+merge/14565 <barry> sinzui: sure, sec * barry changed the topic of #launchpad-reviews to: salgado,barry || reviewing: -,sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com <sinzui> beuno: ping <beuno> sinzui, pong <sinzui> beuno: I am having last minutes doubts about https://bugs.edge.launchpad.net/launchpad-registry/+bug/400643 <sinzui> We agreed to do <sinzui> Ubuntu >> 10.04 >> "bzr" source package <sinzui> But I think it should be <sinzui> Ubuntu >> Lucid (10.04) >> "bzr" source package <beuno> sinzui, I prefer that, yes <sinzui> oh actually, I see examples of this: 6.10 (Edgy) <sinzui> beuno: What is the official way to present version and code name for series? products are 1.23 "code name" <beuno> that's a good question <sinzui> beuno: My example is bad, that was a milestone <sinzui> products use bzr 2.1 series <beuno> Karmic (9.10) or 9.10 (Karmic) <beuno> I think those would be the choices <beuno> I don't like the quotes really <sinzui> Lucid LTS <barry> sinzui: r=me <sinzui> We have yet to accommodate LTS in the name <sinzui> thanks barry <beuno> sinzui, is LTS part of the string? <barry> beuno, sinzui i can't exactly say why, but i think Karmic (9.10) looks better <beuno> barry, it's more readable <sinzui> barry" We like words more than numbers, which is what this bug is really about <beuno> starting with numbers is hard to read <beuno> and hard to compare <barry> beuno sinzui is this a generic change though? I'm not sure I'd like to see (for Mailman): Vital Signs (3.0.0a4) <barry> well, maybe that doesn't looks so bad <beuno> sinzui, I'd just bite the bullet, and it expose it as "CodeName (name)" everywhere <sinzui> beuno: I was going to suggest that. I may need more than one branch, but it does not require me to think. just jfdi <beuno> sinzui, +1 <sinzui> barry: I was pondering a change to series. Use a mix to define all things common to all kinds of series. define title as a combination of version and codename and enforce universal consistentcy <barry> sinzui: that makes sense, as long as there is a code name. of course we can't require a code name (i.e. your rc patch) <sinzui> distroseries have a title attribute that users must provide. product series to not have a codename or a displayname. <sinzui> This problem is really the same scope as fixing all the name, displayname, title, summary, description attributes on all objects <barry> sinzui: those should really be in a common interface <sinzui> Hes they should <sinzui> yes <sinzui> Instead of a UI fix next year, we should do a model fix. Solve the project and projectgroup problem <barry> sinzui: +1 * barry changed the topic of #launchpad-reviews to: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com #launchpad-reviews 2009-11-10 * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: sinzui || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com * bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: sinzui || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: lunch || queue [] || This channel is logged: http://irclogs.ubuntu.com * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: lunch || queue [] || This channel is logged: http://irclogs.ubuntu.com <al-maisan_> stub: I'd appreciate it if you could look at https://code.edge.launchpad.net/~al-maisan/launchpad/builddsc-478919/+merge/14689 at your earliest convenience. <al-maisan_> stub: this patch should be much more readable as I promised the last time ;) <stub> :-) <stub> al-maisan_: Will there be BuildQueue records that reference a Job that does not have a corresponding BuildPackageJob record? <al-maisan_> stub: no, they always come in pairs <al-maisan_> Job and BuildPackageJob always come in pairs that is <stub> It is probably better to link BuildQueue to BuildPackageJob then rather than directly to Job. That will enforce that better. <al-maisan_> stub: the issue here is that there will be other types of jobs (in addition to BuildPackageJob) later <stub> Ok. So in the future a BuildQueue record may be linked to a Job without a BuildPackageJob. <al-maisan_> that's why BuildQueue refers to Job as opposed to some of these specific rows <al-maisan_> stub: there will be e.g. a combination of Job and BuildFromBranchJob <al-maisan_> .. or of Job and RosettaJob <al-maisan_> hance BuildQueue refers to Job and not to any of: BuildPackageJob, BuildFromBranchJob or RosettaJob <al-maisan_> directly <stub> RosettaJob sounds like a dodgy table name, but we can worry about that when it is suggested. Patch looks good. <al-maisan_> stub: 'RosettaJob' was just an example. Thank you for reviewing it! * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com <leonardr> gmb or bac, i have a trivial branch for you: https://code.edge.launchpad.net/~leonardr/lazr.restful/474522-unmarshall-uri/+merge/14691 <bac> leonardr: i can take it * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com <bac> leonardr: done. thanks. * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,- || queue [] || This channel is logged: http://irclogs.ubuntu.com <allenap> gmb: Fancy a review? :) https://code.edge.launchpad.net/~allenap/launchpad/ec2-parry/+merge/14693 <allenap> gmb: There are 4 diffs in that review. If you could do one of them I'd be mucho grateful. <gmb> allenap: Sure, I'll take one. <gmb> allenap: Wouldn't it be easier to use bzr pipeline to split it up into different branches, or is the change pretty much atomic? <gmb> Anyway, I'll take the first diff to start with. * gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: allenap-1, - || queue [allenap-2, allenap-3, allenap-4] || This channel is logged: http://irclogs.ubuntu.com <allenap> gmb: I probably should. I'm happy to do it if you want?
{'channel': 'launchpad-reviews', 'month': 11}