rockstar | mwhudson, around you around to take a branch? | 04:30 |
---|---|---|
rockstar | Er, take==review | 04:30 |
mwhudson | rockstar: hmm, maybe | 04:30 |
mwhudson | rockstar: how big? | 04:31 |
rockstar | mwhudson, it's not very big. One new method on IBranch. | 04:31 |
mwhudson | rockstar: ok then | 04:31 |
rockstar | mwhudson, here's the diff: http://pastebin.ubuntu.com/304776/ | 04:36 |
rockstar | mwhudson, and here's the mp: https://code.edge.launchpad.net/~rockstar/launchpad/ibranch-upgrade/+merge/14201 | 04:36 |
mwhudson | rockstar: well that was easy | 04:40 |
rockstar | mwhudson, I aim to please. | 04:41 |
rockstar | mwhudson, if I change that assert, I can probably remove the other assert as well, ent? | 04:42 |
mwhudson | rockstar: actually, the assert probably won't work as is | 04:45 |
mwhudson | rockstar: i think you probably want assertEqual([job], list(getUtility(IBranchUpgradeJobSource).iterReady())) | 04:45 |
mwhudson | rockstar: if you can get something like that working then yes, you don't need the other assert | 04:45 |
=== Ursinha is now known as Ursinha-afk | ||
wgrant | noodles775, al-maisan: Can I grab one of you to look at https://code.edge.launchpad.net/~wgrant/launchpad/bpr-ddeb_package-db/+merge/14008? | 09:21 |
al-maisan | hello wgrant, is it something that needs to land today? | 09:22 |
al-maisan | It's Friday of week 3 :P | 09:22 |
noodles775 | wgrant: debug_package +1 | 09:22 |
wgrant | noodles775: OK. I was following bigjools' advice with ddeb_package. | 09:23 |
noodles775 | wgrant: although, I'm guessing Julian would agree with your original name... | 09:23 |
noodles775 | wgrant: yes, exactly | 09:23 |
wgrant | al-maisan: Hm, forgot it was that late, sorry. | 09:23 |
al-maisan | wgrant: no problem .. it is just that I really need to land 2 branches myself today. | 09:24 |
noodles775 | wgrant: so although i'd prefer debug_package, I think we can assume that julian would go for ddeb_package, but if it doesn't need to land now, he'll be around next week to confirm. | 09:24 |
wgrant | noodles775: It doesn't need to land this cycle, as the rest won't. | 09:24 |
wgrant | So I guess I shall confirm with him. | 09:25 |
wgrant | Sorry for the noise. | 09:25 |
al-maisan | hello adeuring, I am in dire need of a review (https://code.edge.launchpad.net/~al-maisan/launchpad/newuris-399186/+merge/14204) | 09:32 |
al-maisan | would you be willing to look at the branch above? | 09:32 |
adeuring | al-maisan: review just done :) r=me | 09:32 |
al-maisan | adeuring: thanks .. but this another branch :P https://code.edge.launchpad.net/~al-maisan/launchpad/newuris-399186/+merge/14204 | 09:33 |
adeuring | al-maisan: Ah, i'll look at it | 09:33 |
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: al-maisan || queue: [leonardr, maxb, salgado] || This channel is logged: http://irclogs.ubuntu.com/ | ||
* al-maisan sends virual cookies to adeuring | 09:34 | |
al-maisan | *virtual | 09:34 |
adeuring | al-maisan: could you add a doc string for the new method traverse() (diff line 21) | 09:39 |
al-maisan | adeuring: will do. | 09:40 |
adeuring | al-maisan: in ArchivePermissionSet._nameToPackageSet() you are using ubuntu.currentseries as the series. Is there no use case where this method should search for a package in another series? | 09:52 |
adeuring | ...i mean search for a pkg set | 09:52 |
al-maisan | adeuring: the "problem" here is that the user only specified a package set *name* which is not enough to find a package set | 09:53 |
adeuring | al-maisan: right. | 09:53 |
al-maisan | in other cases where the user does not specify the distro series for the package set we use `currentseries` as a default | 09:53 |
al-maisan | using it here is hence consistent with the rest of the package set API | 09:54 |
al-maisan | if they want a package set in a different series they can pass in a `IPackageset` instance as opposed to a name | 09:55 |
adeuring | al-maisan: Ah, now I undertsnd! thanks | 09:55 |
al-maisan | in which case there is no need to second guess their intentions :) | 09:55 |
al-maisan | yup | 09:55 |
BjornT_ | adeuring: could you review https://code.edge.launchpad.net/~bjornt/launchpad/windmill-more-problems/+merge/14172? i didn't add it to the irc topic yesterday, since i wouldn't be around for the review. but if i would have added it, i would have been first in the queue :) | 10:00 |
adeuring | BjornT_: sure | 10:00 |
BjornT_ | thanks. it should be quite quick, it's only changes to tests | 10:01 |
adeuring | al-maisan: s/aleady/already/ in the added tests. | 10:05 |
adeuring | al-maisan: r=me | 10:05 |
al-maisan | adeuring: thanks! Will fix that. | 10:05 |
al-maisan | that = typo | 10:06 |
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: BjornT_ || queue: [leonardr, maxb, salgado] || This channel is logged: http://irclogs.ubuntu.com/ | ||
=== adeuring changed the topic of #launchpad-reviews to: adeuring || reviewing: salgado || queue: [leonardr, maxb] || This channel is logged: http://irclogs.ubuntu.com/ | ||
danilos | henninge, jtv: anyone of you have time to take a look at my branch for #459132 (350 lines)? | 11:17 |
mup | Bug #459132: Clean up existing untranslated credits messages <Launchpad Translations:In Progress by danilo> <https://launchpad.net/bugs/459132> | 11:17 |
jtv | danilos: I don't, sorry! | 11:17 |
danilos | jtv, ok, that's a shame :) let's see if henning does or will I have to go for a slot in the queue :) | 11:18 |
henninge | danilos: will look, if you look at mine ;-) | 11:18 |
jtv | danilos: queue's pretty full, so I'd take henning's offer :) | 11:18 |
danilos | henninge, sounds good, diff line for line :) | 11:18 |
danilos | jtv, it's not like I haven't noticed :) | 11:19 |
jtv | that's why you asked in the first place... got it. | 11:19 |
henninge | danilos: although I am liable to drop out at any time ... ;-) | 11:21 |
henninge | danilos: here is my mp, https://code.edge.launchpad.net/~henninge/launchpad/bug-461756/+merge/14211 | 11:21 |
danilos | henninge, mine is at https://code.edge.launchpad.net/~danilo/launchpad/bug-459132/+merge/14210 | 11:21 |
henninge | I was about to ask ... | 11:21 |
danilos | henninge, just one question about yours: is there a reason not to default to ordered_by_names=True? | 11:24 |
henninge | danilos: the risk of breaking other code/tests. That is the only reason. | 11:25 |
henninge | backward compatibility | 11:25 |
danilos | henninge, right, it'd be nice to see how much stuff breaks, but not that important right now | 11:25 |
danilos | henninge, it looks good, a very nice branch, r=danilo :) | 11:26 |
henninge | danilos: thank you | 11:26 |
danilos | henninge, judging by the complexity of our branches here, I'd say you didn't get the better of it :) | 11:28 |
henninge | danilos: I wasn't expecting that ... :-/ | 11:29 |
danilos | henninge, it's not too bad I hope :) | 11:33 |
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [leonardr, maxb] || This channel is logged: http://irclogs.ubuntu.com/ | ||
adeuring | leonardr: I see that you are in the review queue but i can't find any MP from you | 11:36 |
leonardr | adeuring: i'll look it up | 11:36 |
leonardr | you probably dont' see it because it's a launchpadlib branch | 11:36 |
leonardr | https://code.edge.launchpad.net/~leonardr/launchpadlib/trusted-workflow-tests | 11:36 |
leonardr | er, https://code.edge.launchpad.net/~leonardr/launchpadlib/trusted-workflow-tests/+merge/14167 | 11:37 |
=== jtv is now known as jtv-afk | ||
adeuring | topic on call: adeuring || reviewing: leonardr || queue: [maxb] || This channel is logged: http://irclogs.ubuntu.com/ | 11:38 |
henninge | danilos: no, it's fine. After all, I am familiar with the stuff. | 11:39 |
henninge | danilos: I see you use the storm "In" operator in pofile.py. I had tried to use it in potemplate.py which your branch touches, too. | 11:40 |
henninge | danilos: it didn't work some how. You left the sql code in there (line 85), did you try to use storm In here, too? | 11:41 |
danilos | henninge, I haven't tried, most likely | 11:41 |
danilos | henninge, I can try it and it should probably work | 11:42 |
henninge | danilos: would be nice. I cannot remember why I couldn't get it to work. | 11:43 |
danilos | henninge, right, I'm trying it out | 11:44 |
danilos | henninge, doesn't work for me either: "NameError: global name 'In' is not defined" :) ok, ok, I'll add an import as well :) | 11:44 |
danilos | henninge, doc/potemplate.txt passes if I use In() there | 11:45 |
danilos | henninge, http://paste.ubuntu.com/304971/ | 11:46 |
henninge | danilos: also plase fix the docstrig for CreditsFixer which was copied from the statistics script. | 11:47 |
henninge | danilos: very nice, thank you | 11:48 |
danilos | henninge, how could you tell? :) | 11:48 |
henninge | :) | 11:49 |
adeuring | leonardr: you are raising RequestTokenAlreadyAuthorized(self.message(self.NONEXISTENT_REQUEST_TOKEN) -- the combintaion of parameter name/content and the exception name looks a bit odd ;) What about using TokenAuthorizationException? | 11:52 |
leonardr | adeuring: ok. the most likely reason the token woudln't exist is that it had already been authorized and exchanged | 11:53 |
leonardr | i'm actually thinking of using TokenAuthorizationException for everything | 11:53 |
leonardr | i don't know if i need all those exception types | 11:53 |
adeuring | leonardr: OK, could you either add a comment to the existing variant or switch to TokenAuthorizationException ? | 11:55 |
leonardr | i will either switch to TAE or create a new kind of exception | 11:55 |
henninge | danilos: your tests in test_potmsgset.py don't need to call setTranslationCreditsToTranslated explicitely because it is called implicitely now when the POTMsgSet is created. | 11:55 |
=== salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue: [leonardr, maxb] || This channel is logged: http://irclogs.ubuntu.com/ | ||
salgado | adeuring, are you reviewing any of the branches from the queue? | 11:59 |
danilos | henninge, right, which means that the test is a bit flakey; how would I test that with the existing data, when there already is a translator-credits message with diverged, imported, current translation, it works correctly? | 11:59 |
adeuring | salgado: yes; acutally, I'm reviewing leonard's branch, and reviewed maxb's branch already... | 12:00 |
danilos | henninge, i.e. how do I work around that feature of the POTMsgSet creation to be able to test with data as it exists today in LP | 12:00 |
adeuring | on call: adeuring,salgado || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | 12:00 |
henninge | danilos: yeah, it would require a flag all the way down to where the POTMsgSet is created to *not* add the dummy translation. | 12:01 |
adeuring | leonardr: a suggestion: you are showing several messages with a simple "print message". Would it make sense to mode the print statement into its own method showMessage() so that people who write GUI client have a single method they need to oberload? | 12:01 |
=== salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: leonardr,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
adeuring | s/mode/move/ | 12:02 |
henninge | danilos: you could add that flag and remove it later once the script has run. | 12:02 |
henninge | danilos: is there no relvant case in the test data? | 12:02 |
leonardr | adeuring: i do that in the next branch | 12:02 |
maxb | adeuring: thanks - will you take care of PQMing it for me? | 12:02 |
adeuring | leonardr: ok, cool | 12:02 |
danilos | henninge, there might be, but I hate using sampledata for tests :) | 12:02 |
adeuring | maxb: sure, will do that. | 12:03 |
henninge | danilos: then we need the flag, which isn't pretty either since it is only transitional. | 12:03 |
danilos | henninge, though, in this particular case, that sounds better than a flag | 12:03 |
henninge | danilos: I'd say, if there is the right data. Creating new sample data would be dumb, too. | 12:04 |
danilos | henninge, of course | 12:05 |
henninge | danilos: if you'll excuse me, I have a stand-up call now ... ;-) | 12:05 |
salgado | adeuring, can you run 'make check_db_merge' on a pristine devel branch and tell me if it fails? | 12:06 |
danilos | henninge, there are 3 translation credits messages in LP, that should do it | 12:06 |
danilos | henninge, heh, right :) | 12:06 |
adeuring | salgado: just a second... | 12:06 |
adeuring | leonardr: r=me | 12:06 |
adeuring | on call: adeuring,salgado || reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | 12:07 |
salgado | adeuring, that's not updating the topic. ;) | 12:07 |
adeuring | arghhh | 12:08 |
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
adeuring | net my best day.... | 12:08 |
salgado | adeuring, did you get around to running that check_db_merge thing? | 12:14 |
adeuring | salgado: just ran it: no problems | 12:15 |
adeuring | (had to make coffe first...) | 12:15 |
salgado | it fails for me on a mainline branch | 12:15 |
salgado | I must have a conflict in a sourcecode/ branch or something | 12:16 |
intellectronica | adeuring: can you review a patch for me? it's 584, but all removals :) | 12:19 |
intellectronica | adeuring: it's the patch for killing the top-level +filebug | 12:19 |
adeuring | intellectronica: I'd like to start some work on a small branch myself, so I'd prefer it if you could ask salgado too ... | 12:20 |
salgado | sure! I love removals | 12:20 |
intellectronica | adeuring: np | 12:20 |
intellectronica | salgado: great! http://pastebin.ubuntu.com/304994/ | 12:20 |
salgado | intellectronica, why is it being removed? | 12:21 |
intellectronica | salgado: for a long time we thought that it's not really useful, but after changing the workflow for filing ubuntu bugs we needed to do something about filing them from the top-level. we decided that we're just going to kill it | 12:22 |
salgado | intellectronica, ok. and how about the tests removed from 'lib/lp/bugs/stories/guided-filebug/xx-filing-security-bugs.txt'? are aren't we losing some coverage by removing that? | 12:25 |
intellectronica | salgado: no, these tests only cover filing security bugs from the top-level (the test is inappropriately named). filing security bugs from pillar contexts is well covered elsewhere | 12:26 |
salgado | intellectronica, cool. r=me then | 12:27 |
intellectronica | salgado: thanks! | 12:27 |
=== mrevell is now known as mrevell-lunch | ||
=== sinzui changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ | ||
=== mrevell-lunch is now known as mrevell | ||
henninge | danilos: review sent | 13:27 |
danilos | henninge, thanks | 13:27 |
sinzui | barry: stand-up in 2 minutes | 13:30 |
=== noodles775 changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [sinzui, noodles] || This channel is logged: http://irclogs.ubuntu.com/ | ||
noodles775 | adeuring or salgado: could one of you look at this trivial branch: https://code.edge.launchpad.net/~michael.nelson/launchpad/458200-p3a/+merge/14216 | 14:33 |
noodles775 | it's mostly tests. | 14:33 |
adeuring | noodles775: I'm just writing am MP myself; when that's done, I'll look at it. | 14:34 |
noodles775 | Thanks! | 14:34 |
salgado | + self.libraryfile.expires = datetime.utcnow().replace(tzinfo=pytz.utc) | 15:01 |
salgado | adeuring, why do you do that instead of "datetime.now(pytz.UTC)"? | 15:01 |
adeuring | salgado: this gives an error when the value is dtpred in the DB. I found the replace() call in several places; simply copied it | 15:03 |
adeuring | s/dtpred/stored/ | 15:03 |
salgado | hmm | 15:04 |
salgado | that's weird | 15:05 |
adeuring | salgado: seems that I was a bit confused... I'll use datetime.now(pytz.utc) | 15:05 |
salgado | + >>> libraryfile.expires <= datetime.utcnow().replace(tzinfo=pytz.utc) | 15:06 |
salgado | adeuring, you can use that in the test too | 15:06 |
adeuring | salgado: right | 15:06 |
salgado | and I think pylint is on crack, so there's no need to ignore the warning there | 15:07 |
adeuring | salgado: perhaps; so you think I should removed the suppression again, so that we know that we should send pylint to a drug therapy ;)? | 15:08 |
salgado | adeuring, that's what I do in these cases, but it might be time for heavier drugs as I've been seeing this for quite some time now | 15:09 |
adeuring | salgado: ;) OK, removed f0401 | 15:09 |
=== salgado is now known as salgado-lunch | ||
deryck | adeuring or salgado, can I get in queue for a simple lint clean up branch? | 15:13 |
adeuring | deryck: sure | 15:13 |
=== deryck changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [sinzui, noodles, deryck] || This channel is logged: http://irclogs.ubuntu.com/ | ||
adeuring | let me just finish another review | 15:14 |
deryck | ok, thanks, adeuring | 15:14 |
sinzui | salgado-lunch: adeuring I have written a replacement lint for gedit-developer-plugins (launchpad uses a version of my lint script from 2002). I think I can adapt my 100% script to launchpad. It uses PEP8.py instead of pylint. | 15:15 |
adeuring | sinzui: nice idea! | 15:16 |
sinzui | adeuring: I meant to say my script is 100% python. it checks python syntax + style, markup (knows about HTML entities), doctests, and plain-text files. | 15:18 |
adeuring | sinzui: sounds really good! | 15:19 |
sinzui | It is better than good, it is fast | 15:19 |
adeuring | noodles775: r=me | 15:20 |
noodles775 | Thanks adeuring | 15:22 |
adeuring | welcome :) | 15:22 |
adeuring | deryck: r=me | 15:28 |
deryck | adeuring, thanks! | 15:28 |
sinzui | adeuring: can you review my blueprint branch? | 15:29 |
adeuring | sinzui: sure | 15:29 |
sinzui | adeuring: https://code.edge.launchpad.net/~sinzui/launchpad/sprint-attendence-1 | 15:29 |
=== adeuring changed the topic of #launchpad-reviews to: adeuring,salgado || reviewing: sinzui,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
sinzui | ^ I had to write a lot of tests before I could add the new feature because there was nothing veriying the current behaviour. | 15:30 |
adeuring | sinzui: r=me | 16:11 |
sinzui | thank | 16:11 |
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
=== salgado-lunch is now known as salgado | ||
sinzui | adeuring: your grammar is correct. My fingers do not like to type suffixes. I type a lot of unconjugated words. | 16:20 |
adeuring | sinzui: ;) | 16:20 |
=== deryck is now known as deryck[lunch] | ||
=== sinzui changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ | ||
=== beuno is now known as beuno-lunch | ||
leonardr | adeuring or salgado, can one of you review https://code.edge.launchpad.net/~leonardr/launchpadlib/trusted-workflow-tests-2/+merge/14230 ? | 17:34 |
salgado | leonardr, I'll take it | 17:35 |
salgado | leonardr, how big is it? | 17:35 |
leonardr | salgado: about 600 lines | 17:36 |
=== deryck[lunch] is now known as deryck | ||
mrevell | Anyone want to review the what's new branch for 3.1.10? Just text changes. https://code.edge.launchpad.net/~matthew.revell/launchpad/whatsnew-3110/+merge/14231 | 17:40 |
mrevell | adeuring, ^^^ ? | 17:41 |
adeuring | mrevell: sure | 17:41 |
mrevell | thanks adeuring | 17:41 |
leonardr | salgado: sorry to run when i have a branch in the queue but i'm really hungry and must go fetch lunch. if you run into a blocker just drop my branch until i get back | 17:41 |
=== leonardr is now known as leonardr-afk | ||
salgado | leonardr-afk, sure, no worries | 17:42 |
adeuring | mrevell: r=me | 17:49 |
mrevell | thanks adeuring! | 17:50 |
=== beuno-lunch is now known as beuno | ||
salgado | leonardr-afk, the diff in your MP has a bunch of conflict markers | 18:04 |
salgado | abentley, don't MPs show when there are conflicts anymore? | 18:04 |
abentley | salgado: The automatic preview diffs don't record whether there are conflicts. | 18:05 |
salgado | abentley, but MPs used to say something like '9 conflicts' in red, at the top, no? | 18:06 |
abentley | salgado: That was generated by an external script, "MAD". | 18:07 |
salgado | oh, right. and we don't use that anymore? | 18:07 |
abentley | salgado: I'm not sure whether it's still running, but the diffs are being generated by LP now. | 18:08 |
=== leonardr-afk is now known as leonardr | ||
leonardr | salgado, the version i'm pushing now has the conflicts resolve | 19:19 |
leonardr | d | 19:19 |
salgado | leonardr, did your push finish? I got conflicts when merging your branch | 19:51 |
leonardr | salgado: yes, version 77 | 19:53 |
salgado | leonardr, it doesn't seem to be there: https://code.edge.launchpad.net/~leonardr/launchpadlib/trusted-workflow-tests-2 | 19:53 |
leonardr | ah, the name is different | 19:54 |
leonardr | pushing now | 19:54 |
leonardr | salgado: try again | 19:55 |
leonardr | salgado: the reason the method names are the way they are is that i'm using python naming standards, not launchpad naming standards | 19:55 |
salgado | leonardr, right, then it's the other methods that ought to be renamed? | 19:56 |
salgado | getSingleCharInput and pressEnterToExit | 19:56 |
salgado | leonardr, also, what's the reason for this change: | 20:01 |
salgado | - self.web_root = web_root | 20:01 |
salgado | + self.web_root = uris.lookup_web_root(web_root) | 20:01 |
=== salgado changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ | ||
salgado | leonardr? | 20:13 |
=== adeuring changed the topic of #launchpad-reviews to: on call: - || reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ | ||
=== salgado is now known as salgado-afk | ||
leonardr | salgado: sorry for delay | 20:39 |
leonardr | uris.lookup_web_root() lets you specify "staging" instead of "https://staging.launchpad.net/" | 20:39 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!