| === matsubara is now known as matsubara-afk | ||
| thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/remove-thumper-from-sample-data/+merge/22189 diff still pending | 00:55 |
|---|---|---|
| mwhudson | thumper: done | 00:57 |
| james_w | thumper/mwhudson: could you take a look at https://code.edge.launchpad.net/~james-w/launchpad/code-imports-for-source-packages/+merge/21821 please? | 00:58 |
| mwhudson | james_w: after lunch ok? | 00:58 |
| james_w | sure | 00:58 |
| mwhudson | cool | 00:58 |
| james_w | I meant to do it as two branches, but got carried away it seems | 00:59 |
| james_w | I can split it up if you like | 00:59 |
| mwhudson | doesn't seem too large | 00:59 |
| james_w | I'm going to do it, as it makes the tests a bit clearer as to what they are doing | 01:02 |
| === wgrant changed the topic of #launchpad-reviews to: on call:|| reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| james_w | hi mwhudson | 02:13 |
| mwhudson | james_w: hello | 02:14 |
| james_w | mwhudson: do you have time to look now? | 02:14 |
| mwhudson | james_w: i guess you'd like me to look at that branch? | 02:14 |
| mwhudson | yeah | 02:14 |
| mwhudson | james_w: can you paste the link again? | 02:14 |
| james_w | if not, then I'll sign off and we can do this another time | 02:14 |
| james_w | sure | 02:14 |
| mwhudson | resume didn't, again | 02:14 |
| james_w | thanks | 02:14 |
| james_w | heh | 02:14 |
| james_w | https://code.edge.launchpad.net/~james-w/launchpad/new-code-import-method-on-branchtarget/+merge/22190 | 02:15 |
| james_w | https://code.edge.launchpad.net/~james-w/launchpad/code-imports-for-source-packages/+merge/22191 | 02:15 |
| james_w | the first is another refactoring-type branch, and then the second flips the switch and adds tests for source package imports | 02:15 |
| mwhudson | james_w: is there an order to them? | 02:15 |
| mwhudson | ah ok | 02:15 |
| mwhudson | james_w: the diff for https://code.edge.launchpad.net/~james-w/launchpad/new-code-import-method-on-branchtarget/+merge/22190 has conflicts | 02:16 |
| james_w | I'm not sure that AssertionError is the right error to be raising | 02:16 |
| mwhudson | um | 02:17 |
| james_w | yeah, I don't see them | 02:17 |
| mwhudson | or at least the mp page claims it has conflicts | 02:18 |
| mwhudson | that's a bit odd | 02:18 |
| james_w | I think it's something to do with the prerequisite branches | 02:18 |
| james_w | yeah, a prerequisite branch has conflicts with devel, fixing that now | 02:22 |
| mwhudson | james_w: it all looks good, apart from a few details on whitespace of all things | 02:27 |
| james_w | great | 02:27 |
| mwhudson | james_w: balls, reviewed the wrong proposal | 02:33 |
| james_w | oops | 02:34 |
| james_w | conflicts fixed | 02:34 |
| mwhudson | james_w: anyway, both branches reviewed | 02:39 |
| james_w | thanks mwhudson. Both fixed as well. | 02:40 |
| mwhudson | james_w: cool | 02:41 |
| mwhudson | james_w: can you land these yourself yet? | 02:41 |
| james_w | I cannot | 02:41 |
| james_w | now I must depart | 02:42 |
| james_w | thanks for your help | 02:42 |
| mwhudson | bye | 02:42 |
| wgrant | I'm guessing that there will be no OCR today, since Bugs is sprinting? | 09:08 |
| === wgrant changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| salgado | wgrant, I see only one branch from you on https://code.edge.launchpad.net/launchpad-project/+activereviews | 12:45 |
| salgado | is the topic here correct, do you have 2 branches for review? | 12:45 |
| === salgado changed the topic of #launchpad-reviews to: on call: salgado || reviewing: - || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === danilo_ is now known as danilos | ||
| === mrevell is now known as mrevell-lunch | ||
| * bigjools waves at salgado | 13:59 | |
| salgado | howdy bigjools! | 14:00 |
| bigjools | salgado: hey dude, how's it going? | 14:01 |
| bigjools | I've got a nice branch for you :) | 14:01 |
| salgado | you soyuz people always do. :P | 14:01 |
| bigjools | this one's pretty easy as soyuz goes | 14:02 |
| salgado | :) | 14:02 |
| bigjools | popped you on the list of reviewers, you should get some email | 14:03 |
| salgado | cool. will get to it in a minute. just need to submit one of my own branches to ec2 | 14:03 |
| bigjools | I should expand the MP description really | 14:03 |
| === salgado changed the topic of #launchpad-reviews to: on call: salgado || reviewing: bigjools || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| bigjools | thanks salgado | 14:03 |
| * bigjools needs food, bbiab | 14:22 | |
| === mrevell-lunch is now known as mrevell | ||
| === salgado changed the topic of #launchpad-reviews to: on call: salgado || reviewing: - || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-lunch | ||
| === salgado is now known as salgado-lunch | ||
| === deryck is now known as deryck[lunch] | ||
| === sinzui changed the topic of #launchpad-reviews to: on call: salgado || reviewing: - || queue: [wgrant,wgrant, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | EdwinGrubbs: can you review https://code.launchpad.net/~sinzui/launchpad/do-not-release/+merge/22239 for me. We need this landed today, or we must ask for an RC next week | 16:32 |
| jtv | henninge: not good English: "# Not matching format specifiers will not be validated." | 16:34 |
| jtv | "Mismatched" would work, or "non-matching." | 16:34 |
| jtv | henninge: about "# Failing is usually propagated by an exception."—that'd be "Failure." And... when does it not cause an exception? | 16:36 |
| jtv | henninge: do you know it's safe to unicode(message_or_exception) in the GettextValidationError constructor? You might trigger an encoding error right there. May be safer to use repr(message_or_exception) instead. | 16:37 |
| jtv | henninge: in the validate_translation docstring, please backquote the reference to GettextValidationError | 16:38 |
| jtv | henninge: | 16:39 |
| jtv | 779+ elif len(translation): | 16:39 |
| jtv | 780+ msg.set_msgstr(translation[0]) | 16:39 |
| jtv | len(translation) > 0 | 16:39 |
| jtv | and, our coding standards require an else. | 16:39 |
| jtv | henninge: in "# Hide gettextpo.error in GettextValidationError."—"hide" doesn't sound right... how about "wrap"? | 16:40 |
| jtv | henninge: finally, "ignore_errors" doesn't seem to mean "ignore errors," but select between exceptions and a bool return code to indicate validation failure. Assuming this is not easy to clean up, can you come up with a better name? | 16:41 |
| EdwinGrubbs | sinzui: sure, I'll review that now. | 16:43 |
| === matsubara-lunch is now known as matsubara | ||
| === gary_poster is now known as gary-lunch | ||
| === salgado-lunch is now known as salgado | ||
| bigjools | salgado: hi there | 17:23 |
| bigjools | nearly done with the branch changes, but I'm struggling to get cancel_url and next_url working, it's completely ignoring them, any advice? | 17:24 |
| salgado | bigjools, that's weird. is it redirecting somewhere or nowhere? | 17:24 |
| bigjools | salgado: nowhere | 17:26 |
| bigjools | salgado: current diff: http://pastebin.ubuntu.com/401913/ | 17:26 |
| salgado | + @property | 17:27 |
| salgado | + def next_url(self): | 17:27 |
| salgado | + # We redirect back to the PPA owner's profile page on a | 17:27 |
| salgado | + # successful action. | 17:27 |
| salgado | + canonical_url(self.context.owner) | 17:27 |
| salgado | bigjools, maybe if you return the value? | 17:27 |
| salgado | ;) | 17:27 |
| bigjools | fuck sake :) | 17:28 |
| bigjools | it's friday ok? | 17:28 |
| bigjools | salgado: so with those changes how's it looking? | 17:30 |
| salgado | bigjools, you know what, I've made this same mistake a couple weeks ago and it took me quite a while to figure out what was wrong. maybe I caught your mistake so quickly because of that | 17:30 |
| bigjools | salgado: it's so easy to do, I really wish Python would warn if you don't return stuff from a @property | 17:31 |
| bigjools | defaulting to None is crap | 17:31 |
| salgado | indeed | 17:31 |
| salgado | bigjools, you can easily teach pyflakes to do that, I think | 17:32 |
| sinzui | salgado: how? | 17:32 |
| * sinzui has a custome pyflakes + pep8 linter and it does not report that case | 17:33 | |
| salgado | sinzui, I might be wrong | 17:33 |
| sinzui | pep8 could be taught though | 17:33 |
| salgado | but I'd expect it'd be possible to hack pyflakes to check for @properties that don't return | 17:34 |
| sinzui | well, I think it would be easier to teach pep8 that pyflakes | 17:34 |
| salgado | if not archive.private: | 17:35 |
| salgado | link.enabled = False | 17:35 |
| salgado | - if archive.status in ( | 17:35 |
| salgado | - ArchiveStatus.DELETING, ArchiveStatus.DELETED): | 17:35 |
| salgado | + if not archive.is_active: | 17:35 |
| salgado | link.enabled = False | 17:35 |
| salgado | bigjools, care to combine those two if blocks? | 17:35 |
| bigjools | salgado: right yep | 17:36 |
| === gary-lunch is now known as gary_poster | ||
| bigjools | salgado: I have to finish now, I'll pop back later. I'm not landing this branch this cycle anyway, I just need to be ready to land it at the start of the next so it can get some beta testing. | 18:02 |
| salgado | bigjools, cool. I've just approved it, btw | 18:02 |
| bigjools | salgado: ok thanks duderino | 18:03 |
| salgado | you're welcome | 18:03 |
| salgado | sinzui, is the one you have queued the one that EdwinGrubbs is reviewing? | 18:04 |
| bigjools | rockstar: do you want to take a final look at https://code.edge.launchpad.net/~julian-edwards/launchpad/ppa-deletion-ui/+merge/21925 ? | 18:04 |
| bigjools | (ppa deletion) | 18:04 |
| sinzui | salgado: I hope so | 18:04 |
| * bigjools runs, goodnight all | 18:04 | |
| EdwinGrubbs | sinzui: r=me | 18:05 |
| sinzui | EdwinGrubbs: thanks? did you have any concerns about the filtering of the obsolete/old packages? | 18:06 |
| sinzui | EdwinGrubbs: I think I may want to adjust to sorting of Product.sourcepackages next month. The order is alphabetical. It kinds of works, but If the primary use case is listing important packages, I think I want to switch to newest first. We will have to change this eventually because we as half way through the alphabet | 18:10 |
| EdwinGrubbs | sinzui: well, I saw that the test determined obsolete by whether there was a spph. I don't know if it would make more sense to check the distroseries status. | 18:28 |
| sinzui | EdwinGrubbs: the series status and is the cause of the no currentrelease. We removed the release. This will fix the old packages that are listed on bzr page | 18:29 |
| sinzui | s/and is/is/ | 18:30 |
| EdwinGrubbs | sinzui: what do you mean by "we removed the release". | 18:31 |
| sinzui | EdwinGrubbs: I took the two properties from the branch that lets users say the project is not packaged. I can fix the sort order when I land that branch. I still am not certain the sort order is wrong. I just think the order can have surprises. | 18:31 |
| sinzui | EdwinGrubbs: We purge the old packages when the series goes obsolete | 18:31 |
| sinzui | EdwinGrubbs: looking at https://edge.launchpad.net/gedit, you can see that every package that is missing a currentrelease is also an obsolete series | 18:32 |
| EdwinGrubbs | sinzui: I think you are right that sorting by date would be better than the current sorting by name. | 18:36 |
| sinzui | right, hardy and hoary look wrong | 18:36 |
| sinzui | That comes from the model. Since it appears to be fore display purposes, I think I will try to change that in my next branch | 18:37 |
| gary_poster | james_w: switching here because I suppose this is the proper thing to do. :-) | 18:49 |
| james_w | I guess :-) | 18:49 |
| gary_poster | james_w: Did you consider AssertionError over ValueError? I was gung ho for AssertionError, but then started convincing myself of ValueError. I still lean towards AssertionError...ooh, no I have an idea I like better, maybe. One sec, looking up. | 18:53 |
| james_w | I don't really mind what the error type is | 18:55 |
| gary_poster | OK, james_w, here is the trivia I'm struggling with. Normally, the right thing to do in this case is to write a function (I'm going to suggest that we switch the class to a function anyway) that returns None. That behaves properly with standard Zope calls: getMultiAdapter will raise a ComponentLookupError, and queryMultiAdapter will return None or the provided default. | 19:05 |
| gary_poster | By explicitly raising an exception we're breaking the pattern. Raising ComponentLookupError (what I was considering) is not appropriate here because that's the responsibility of something higher up. And the reason we are doing this is to help the user figure out what they did wrong. In would be nice if the zope machinery had a spelling for that, but it doesn't. | 19:05 |
| gary_poster | So, I think an AssertionError is the right thing to do for our intentions: we're doing this to help a poor developer debug and fix their problems, which is the domain of an AssertionError. A ValueError is an attempt to raise a "real" error, and would be the wrong "real" error (ComponentLookupError is the right error, if we get one at all). | 19:05 |
| gary_poster | So with all that, I would suggest changing the ChoiceErrorMarshaller class to something like this: | 19:05 |
| gary_poster | def choiceMarshallerError(field, request, vocabulary): | 19:05 |
| gary_poster | # We don't support marshalling a normal Choice field with a | 19:05 |
| gary_poster | # SQLObjectVocabularyBase-based vocabulary. | 19:05 |
| gary_poster | # Normally for this kind of use case, one returns None and | 19:05 |
| gary_poster | # lets the Zope machinery alert the user that the lookup has gone wrong. | 19:05 |
| gary_poster | # However, we want to be more helpful, so we make an assertion, | 19:05 |
| gary_poster | # with a comment on how to make things better. | 19:05 |
| gary_poster | assert False, (...your message...) | 19:05 |
| james_w | ok | 19:06 |
| gary_poster | james_w: other than that, r=gary. You cool with that? | 19:06 |
| james_w | yeah, let me make the changes so that you can confirm | 19:07 |
| gary_poster | ok cool | 19:07 |
| james_w | gary_poster: please re-check | 19:15 |
| gary_poster | ack james_w | 19:15 |
| gary_poster | james_w: +1. I really would like (something like) the comment I gave to be there too. People who know the Zope pattern will appreciate reading what's going on (like me, when I've forgotten all about this!) | 19:17 |
| james_w | oh | 19:17 |
| gary_poster | Other than that, that's what I had in mind. I need to step out to the bank. Would you like me to give you a merge-conditional approve so you can proceed without me? | 19:18 |
| james_w | done | 19:18 |
| james_w | I can't merge anyway, so there's no rush | 19:19 |
| gary_poster | heh, ok | 19:19 |
| === EdwinGrubbs is now known as Edwin-lunch | ||
| gary_poster | you might not have pushed the branch, James, but I gave a merge-conditional | 19:22 |
| james_w | it was just waiting for the puller, it's there now | 19:24 |
| === gary_poster is now known as gary-afk | ||
| === gary-afk is now known as gary_poster | ||
| === gary_poster is now known as gary-afk | ||
| henninge | jtv: http://paste.ubuntu.com/401997/ | 20:05 |
| === sinzui changed the topic of #launchpad-reviews to: on call: salgado || reviewing: - || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary-afk is now known as gary_poster | ||
| === matsubara is now known as matsubara-afk | ||
| === Edwin-lunch is now known as EdwinGrubbs | ||
| === adiroiban changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant,wgrant,adiroiban(bug-548999)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!