=== abentley1 is now known as abentley | ||
=== ursula is now known as Ursinha | ||
=== jtv changed the topic of #launchpad-reviews to: on call: — || reviewing: — || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com | ||
=== mrevell is now known as mrevell-lunch | ||
=== bigjools is now known as bigjools-phone | ||
stub | rockstar: Do you want to review https://code.edge.launchpad.net/~stub/launchpad/replication/+merge/13203 since it is your bug? | 13:09 |
---|---|---|
=== bigjools-phone is now known as bigjools | ||
=== mrevell-lunch is now known as mrevell | ||
=== EdwinGrubbs2 is now known as EdwinGrubbs | ||
=== barry` is now known as barry | ||
=== barry` is now known as barry | ||
=== stub1 is now known as stub | ||
=== intellec` changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: — || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com | ||
=== intellec` changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: adeuring || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com | ||
=== intellec` is now known as intellectronica | ||
intellectronica | adeuring: why `device['E'].get('SUBSYSTEM')` and not `device['E']['SUBSYSTEM']` ? | 16:20 |
intellectronica | adeuring: or is it not a get from a dictionary-like object? | 16:20 |
adeuring | intellectronica: it is a dict. Let me check, if we can guaraty if SUBSYSTEM is always defined.... | 16:21 |
intellectronica | right | 16:21 |
adeuring | intellectronica: no, SUBSYSTEM is _not_ always defined. | 16:22 |
adeuring | intellectronica: to reproduce: udevadm info --export-db | 16:22 |
intellectronica | ah ok, so get might return None | 16:22 |
adeuring | intellectronica: exactly | 16:22 |
intellectronica | I'd supply the extra parameter, just for readability | 16:22 |
adeuring | intellectronica: do you mean a sort of an explicit marker? | 16:23 |
intellectronica | adeuring: device['E'].get('SUBSYSTEM', None) | 16:23 |
adeuring | intellectronica: hrmm... I am not generally opposed to that, but IÄ'd gues that other reviewers might suggest to remove the second parameter ;) | 16:24 |
intellectronica | adeuring: the docstring of is_scsi_device can terminate on the first line, no? | 16:24 |
adeuring | intellectronica: right, changed | 16:24 |
intellectronica | adeuring: you think so? i'll leave it for you to decide, but i think explicit is better than implicit | 16:24 |
adeuring | intellectronica: as I said, I don't have a definite opinion on this. Something for the reveiwer meeting? | 16:25 |
intellectronica | sure, or we can even ask barry what he thinks | 16:26 |
* barry thinks it's all wrong! oh wait... | 16:27 | |
intellectronica | i don't feel strongly about this, it meant literally what i would do | 16:27 |
intellectronica | barry: abel writes dict.get('sumfin') and i like dict.get('sumfin', None) | 16:28 |
intellectronica | because i always forget what the default behaviour is | 16:28 |
barry | intellectronica: i don't think you need that. every python developer knows .get() returns None by default if the key is missing :) | 16:28 |
barry | intellectronica: that's deeply ingrained in python dna :) | 16:28 |
barry | however, if it's possible that 'sumfin' is in the dict with a value of None, then it's much better to do something like: | 16:29 |
barry | missing = object() | 16:29 |
barry | dict.get('sumfin', missing) | 16:29 |
adeuring | barry: that's not the case here | 16:29 |
barry | adeuring: cool. in that case .get('foo') is fine | 16:29 |
intellectronica | adeuring: you see, i'm talking nonsense. clearly i'm not (genetically speaking) a python developer :) | 16:30 |
adeuring | intellectronica: ;) the point is that you can deevelop Python genes ;) | 16:30 |
barry | intellectronica: too much javascript will rot your brain | 16:30 |
intellectronica | adeuring: other than that docstring it all looks great. r=me | 16:35 |
adeuring | intellectronica: thanks! | 16:35 |
=== intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com | ||
=== intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
nhandler | Does anyone know what file sets the 'Invalid value' error message for the subscription fields? | 16:38 |
=== intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
intellectronica | bigjools: would you like me to review first-ppa-name-bug-439305 ? | 16:39 |
bigjools | intellectronica: ah yes please, forgot to ask about that! I have a couple possible changes though, perhaps I can discuss them with you after you've looked at it | 16:40 |
intellectronica | bigjools: sure | 16:41 |
=== andrea-bs_ is now known as andrea-bs | ||
intellectronica | bigjools: your branch looks fine to me. what changes did you want to make? | 16:49 |
bigjools | first, I was considering filling the "name" field with "ppa" if it's the first PPA | 16:50 |
bigjools | so it's a sensible default | 16:50 |
bigjools | second, I might change the interface description to make it more obvious that the name will be part of the repo URL | 16:50 |
bigjools | and also possibly change the displayname description so the user knows it will be part of the GPG key descriptiojn | 16:51 |
intellectronica | bigjools: yes, all three changes sound like very good ideas | 16:55 |
bigjools | intellectronica: okay, thanks, do you want to see another diff with that in? | 16:56 |
intellectronica | bigjools: ye | 16:58 |
intellectronica | yes | 16:58 |
bigjools | coming right up | 16:58 |
bigjools | intellectronica: what's the right way of pre-filling a form field? | 16:59 |
intellectronica | bigjools: in the view, default_values is a dict with an entry for every field you want to pre-fill | 17:00 |
bigjools | cool, ta | 17:00 |
bigjools | intellectronica: hmm is it possible to conditionally change the field descriptions on the form? part three above only applies to the first created PPA | 17:05 |
intellectronica | bigjools: yes, but it's hackish. you need to create a new field, and then create a new fields object with the original field omitted and the new one inserted instead of it | 17:06 |
bigjools | urg | 17:07 |
bigjools | IIRC there is another way to add more text on the form? | 17:07 |
intellectronica | bigjools: nothing i can think of | 17:08 |
noodles775 | intellectronica: I'm guessing you'll be finishing soon too, but if I'm wrong, there'll be an MP of mine arriving in a minute if you've time. | 17:08 |
intellectronica | maybe ask one of the resident zope mavens? | 17:08 |
intellectronica | noodles775: i can fit one more review | 17:09 |
noodles775 | intellectronica: thanks! Note - I won't be around, so feel free to re-prioritise :) | 17:09 |
=== noodles775 changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com | ||
intellectronica | noodles775: np | 17:09 |
* bigjools looks for a maven | 17:10 | |
bigjools | I think they all have a holiday | 17:11 |
=== intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: noodles || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
intellectronica | bigjools: i think gary_poster is around | 17:21 |
bigjools | intellectronica: default_values didn't work | 17:22 |
intellectronica | maybe it's called something different? let me find an example for you | 17:23 |
bigjools | thanks | 17:23 |
intellectronica | bigjools: sorry, i've misled you. it's actually called initial_values. see lib/lp/bugs/browser/bugtarget.py for example | 17:24 |
bigjools | intellectronica: got it, thanks | 17:25 |
gary_poster | bigjools: not sure what I'm a maven of, but I am around. I am kind of trying to sneak lunch in though. But if you ask me something or other I'll try to get back quickly | 17:25 |
bigjools | gary_poster: real quick one! is there a way to conditionally get some extra text on a form, or preferably change the value of a form field's description | 17:26 |
=== EdwinGrubbs is now known as Edwin-lunch | ||
intellectronica | gary_poster: i suggested creating a new field and and a new fields collection with the new field replacing the old one, but we didn't know if there's an easier way to do it | 17:27 |
gary_poster | bigjools, intellectronica: what intellectronica said sounds like the right basic idea. it's possible we are already creating a one-off field collection for the request, in which case you can just change it in place, which would make it shorter, at least. | 17:31 |
gary_poster | but that would just be an optimization of the same basic idea | 17:31 |
bigjools | gary_poster: is there a property on the field I can poke then? | 17:32 |
* bigjools boggles at the number of requests in .dev for a simple page | 17:34 | |
bigjools | intellectronica: is there a trick to getting the initial_values property called? Mine isn't :/ | 17:35 |
bigjools | oh fuck | 17:35 |
bigjools | I need to spell it right of course | 17:35 |
intellectronica | noodles775: r=me. it would be great to convert that sqlo query to storm, but that's beyond the scope of the branch. everything else looks great | 17:39 |
bigjools | heh the BUDQ | 17:41 |
intellectronica | ? | 17:42 |
=== intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
bigjools | Big Ugly Dispatcher Query | 17:45 |
bigjools | intellectronica: http://pastebin.ubuntu.com/291713/ | 18:02 |
intellectronica | bigjools: i suspect that the way you've written initial_values, the value will be overridden even if filled by the user before submissions (say if there's an error in another field). is that the behaviour you want? | 18:04 |
bigjools | intellectronica: I don't understand | 18:04 |
intellectronica | bigjools: if you submit the form and make an error, the form is displayed again with the values you've used before. if you always override initial_values, the previously entered value won't be retained (which is the default behvaiour), because you're looking at the value on the context, which hasn't changed | 18:06 |
bigjools | ah i see | 18:06 |
intellectronica | bigjools: i suspect that's what wil happen. i'm not 100% sure, but i haven't tried | 18:06 |
bigjools | I'll check it out and see what happens | 18:07 |
intellectronica | cool | 18:07 |
intellectronica | everything else looks fine. i take you've decided that manipulating the field's description dynamically is to much of a hassle? | 18:07 |
bigjools | yes, the return on investment is small | 18:08 |
intellectronica | i agree | 18:08 |
bigjools | intellectronica: actually the value is retained, despite initial_values | 18:09 |
bigjools | so it's all good! | 18:09 |
intellectronica | bigjools: excellent. r=me then | 18:09 |
bigjools | intellectronica: great, thanks for your advice and review | 18:09 |
intellectronica | and with that i shall end my review shift. i think i've been more trouble than help as a reviewer today :-/ | 18:10 |
bigjools | hehe | 18:10 |
=== intellectronica changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
bigjools | fud time here | 18:10 |
=== bigjools is now known as bigjools-afk | ||
gary_poster | bigjools-afk: if you can get to a widget then you are home free: set the .label or the .hint, depending on what you want. | 18:25 |
gary_poster | if that doesn't do what you want, back to the drawing board. You can do a custom widget easily-ish. | 18:26 |
=== EdwinGrubbs2 is now known as EdwinGrubbs | ||
leonardr | gary, can you review this tiny lazr.restful diff so i can make a new release for launchpad? | 21:46 |
leonardr | https://pastebin.canonical.com/23226/ | 21:46 |
gary_poster | yes | 21:46 |
leonardr | i've also got a small launchpad branch coming up. (my ec2 test just passed) | 21:46 |
gary_poster | leonardr: r=gary | 21:46 |
gary_poster | great | 21:46 |
leonardr | can i commit my changes to the download cache? | 21:47 |
gary_poster | leonardr: yes | 21:47 |
leonardr | all right | 21:47 |
leonardr | i love it when a plan comes together | 21:47 |
gary_poster | :-) | 21:47 |
leonardr | gary: fyi, here's what i'm adding | 21:48 |
leonardr | added: | 21:48 |
leonardr | dist/grokcore.component-1.6.tar.gz | 21:48 |
leonardr | dist/lazr.restful-0.9.11.tar.gz | 21:48 |
leonardr | dist/martian-0.11.tar.gz | 21:48 |
leonardr | dist/wsgiref-0.1.2.zip | 21:48 |
gary_poster | leonardr: cool | 21:48 |
leonardr | gary: https://code.edge.launchpad.net/~leonardr/launchpad/help-me-gary/+merge/13245 | 22:02 |
gary_poster | heh | 22:04 |
leonardr | that's what it was called when i couldn't figure out what those tarballs were for | 22:04 |
gary_poster | heh, I see | 22:04 |
gary_poster | leonardr: r=gary | 22:07 |
leonardr | all right | 22:08 |
* leonardr has not pqm-submitted a branch for months. does it still work the same way? | 22:08 | |
gary_poster | it can still work with bzr pqm-submit. There are other new tricks, but that should be fine. | 22:08 |
leonardr | all right | 22:08 |
leonardr | i'm off, i'll see you tomorrow | 22:09 |
gary_poster | see you | 22:09 |
gary_poster | thanks | 22:09 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!