| henninge | Hi noodles775! | 09:29 |
|---|---|---|
| noodles775 | Hi henninge :) | 09:31 |
| henninge | ;) | 09:31 |
| === allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | noodles775: could you please have a look at jtv's review UI-wise? https://code.edge.launchpad.net/~jtv/launchpad/translationimportqueueentry-info/+merge/18963 | 09:31 |
| henninge | noodles775: I just attached some screenshots for you to look at. Scroll down to after my code review. ;) | 09:32 |
| noodles775 | henninge: I can, but it'd be great to check first if sinzui was keen (afaiui, he's keen to build up ui reviews). I can then go through it after that if required. | 09:32 |
| henninge | s/review/merge proposal/ | 09:32 |
| noodles775 | But if it's urgent, I can do it today? | 09:33 |
| henninge | noodles775: will sinzui not be available today? It could wait until he gets up ... | 09:33 |
| henninge | noodles775: according to Matt's mail he should be here, so it can wait. | 09:35 |
| henninge | noodles775: thanks | 09:35 |
| === matsubara-afk is now known as matsubara | ||
| henninge | Hi jpds! I have not yet been able to land your branch. The ec2 test runs kept breaking off. | 10:22 |
| henninge | jpds: can you please merge the current devel into your branch and push it again? | 10:22 |
| henninge | I am talking about lp:~jpds/launchpad/fix_518232, btw. ;-) | 10:23 |
| jpds | henninge: I get http://pastebin.ubuntu.com/373098/ | 10:25 |
| jpds | henninge: On an up-to-date devel. | 10:25 |
| henninge | jpds: sorry, that was the wrong way round ;-) | 10:25 |
| jpds | Ah, I get Nothing to do. | 10:26 |
| henninge | jpds: make sure to revert that merge on devel. "bzr revert" should do it if you did not commit. | 10:26 |
| henninge | jpds: what I meant was this : http://pastebin.ubuntu.com/373099/ | 10:27 |
| jpds | I did bzr revert --no-backup | 10:27 |
| al-maisan | hello allenap, I have an easy branch for you :) | 10:34 |
| al-maisan | https://code.edge.launchpad.net/~al-maisan/launchpad/flip-the-switch-516545/+merge/18992 | 10:34 |
| allenap | al-maisan: Cool, on it :) | 10:34 |
| al-maisan | allenap: thanks! | 10:35 |
| jpds | henninge: All done. | 10:37 |
| henninge | jpds: cool, I'll try landing again. | 10:38 |
| === jtv changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | al-maisan: r=me, with one small comment. | 10:52 |
| al-maisan | allenap: thanks, on the phone. | 10:52 |
| henninge | jpds: ec2 test instance is running as usual this time. Looking good. ;) | 11:11 |
| === mrevell is now known as mrevell-lunch | ||
| jtv | henninge: updated... it did grow the diff some. | 13:25 |
| === mrevell-lunch is now known as mrevell | ||
| === jamalta-afk is now known as jamalta | ||
| henninge | jtv: ping | 14:33 |
| mrevell | allenap, review a branch for me NOW | 14:38 |
| allenap | mrevell: Aye aye. | 14:38 |
| mrevell | allenap, https://code.edge.launchpad.net/~matthew.revell/launchpad/ppa-add-repo-help-492283/+merge/19010 | 14:38 |
| mrevell | ta | 14:38 |
| bigjools | how to win friends and influence reviews, by mrevell | 14:39 |
| mrevell | yessir | 14:39 |
| === allenap changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: mrevell, - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | mrevell: rejected=me, incorrectly formatted cover note. | 14:41 |
| mrevell | allenap, Are you complaing about my TPS reports? | 14:43 |
| allenap | mrevell: Launchpad cannot compete in the marketplace without correct TPS reports. Please sign up for the TPS refresher programme. | 14:45 |
| allenap | mrevell: How the (&&%$%^& do I see this new help on launchpad.dev? | 14:47 |
| jtv | henninge: pong | 14:49 |
| mrevell | allenap, good question, I take your point -- https://launchpad.dev/~cprov/+archive/ppa | 14:57 |
| === matsubara is now known as matsubara-lunch | ||
| === sinzui changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: mrevell, - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| al-maisan | hello jtv, would you be in a position to review a branch of mine? | 15:22 |
| al-maisan | https://code.edge.launchpad.net/~al-maisan/launchpad/disabled-copyarch-519665/+merge/19019 | 15:22 |
| jtv | al-maisan: right after the reviewers meeting and team standup, sure | 15:22 |
| al-maisan | ah | 15:22 |
| al-maisan | oh | 15:22 |
| al-maisan | I missed that.. | 15:22 |
| === al-maisan changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: mrevell, - || queue [sinzui, al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | allenap: should I do sinzui's branch first? | 15:24 |
| allenap | jtv: Okay, sounds good. | 15:25 |
| jtv | al-maisan: so you're in line after curtis then | 15:25 |
| al-maisan | yup | 15:25 |
| === allenap changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: al-maisan, sinzui || queue [sinzui, al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: al-maisan, sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | allenap, jtv: Could you please review https://code.edge.launchpad.net/~abentley/launchpad/forbidden-oops/+merge/19021 ? | 15:37 |
| allenap | abentley: Sure. | 15:37 |
| === allenap changed the topic of #launchpad-reviews to: on call: allenap, jtv || reviewing: al-maisan, sinzui || queue [abentley] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | allenap, thanks. | 15:37 |
| jtv | allenap: I don't see sinzui's MP anywhere... do you? | 15:42 |
| sinzui | jtv: https://code.edge.launchpad.net/~sinzui/launchpad/launchapd-apocalypse-1/+merge/18462 | 15:43 |
| allenap | jtv: There it is ;) | 15:43 |
| jtv | oh, that review's already been claimed | 15:43 |
| jtv | sinzui: do you want it handled by OCR or stick with Edwin/Abel? | 15:44 |
| sinzui | jtv: I really want the branch landed, so please review it. It is very boring. | 15:45 |
| jtv | sinzui: you lost me at "boring" | 15:45 |
| jtv | but alright :) | 15:45 |
| sinzui | jtv: the migrater script create large diffs of mechanical changes | 15:46 |
| jtv | now if only we had a mechanical reviewer... | 15:46 |
| jtv | sinzui: as for the renaming, personally I'm fine with "hwdb" as a prefix (which must be short) but "hardwaredb" as the module name (which must be reasonable but legible). | 15:47 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, jtv, Edwin || reviewing: al-maisan, sinzui || queue [abentley] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | sinzui: if test_views.py is new, not copied over, then please update the copyright year | 15:51 |
| jtv | although this one looks familiar... if it's a copy-paste job, shouldn't we have a helper for this? | 15:51 |
| sinzui | jtv: right, the migrater assumes we did what we claimed we would do in 2009 | 15:51 |
| jtv | sinzui: electric monk? | 15:52 |
| jtv | We're finally writing software to believe things for us that we don't have time to believe ourselves now? | 15:52 |
| sinzui | jtv:I think this is a case where many teams did not take this effort seriously. This issue looks like a large mass of uncompleted work | 15:53 |
| EdwinGrubbs | abentley: I'm starting to review your forbidden-oops branch. | 15:53 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, jtv, Edwin || reviewing: al-maisan, sinzui, abentley || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | sinzui: you mean the migrator? | 15:54 |
| abentley | EdwinGrubbs, allenap already agreed to review it. | 15:54 |
| sinzui | jtv: canonical.launchpad should not exist. | 15:54 |
| jtv | sinzui: your branch brings us a step closer. Maybe file a foundations bug for the duplicate file? | 15:55 |
| sinzui | jtv: it remains the primary cause of circular imports, code that does not follow our style, code that you must hunt to fine, code that you cannot test on the module level for the feature | 15:55 |
| abentley | EdwinGrubbs, I'm fine with you reviewing it, just want to avoid two people reviewing it. | 15:55 |
| EdwinGrubbs | makes sense | 15:56 |
| sinzui | I started another branch that modifies the migrater to support services so that I can move them on mass. | 15:56 |
| EdwinGrubbs | allenap: do you want me to take it. | 15:56 |
| allenap | EdwinGrubbs: If abentley is happy with that, then sure. I'll start it in just a few minutes otherwise; just finishing al-maisan's review. | 15:57 |
| jtv | sinzui: ambiguous parse... sorry to be dense but is "support" a verb in that sentence? | 15:57 |
| jtv | (I could read it either way) | 15:57 |
| EdwinGrubbs | allenap, abentley: I'll start on it now. | 15:57 |
| === matsubara-lunch is now known as matsubara | ||
| sinzui | jtv: many of the unmigrated code should be subtrees in lp/serivces. Each has its own interfaces and models directory | 16:00 |
| jtv | sinzui: oic, that makes sense yes—a lot of the stuff that's still in l.c.l is there for that reason, I think | 16:01 |
| jtv | sinzui: about the review... in lib/lp/hardwaredb/tests/test_doc.py, you're using our long-list notation style for an arguments list. I didn't know you could have a trailing comma there! | 16:02 |
| sinzui | jtv: I think pep8.py demands it to be there. The file was made by the migrater | 16:02 |
| sinzui | of, no it wasn't, I had to edit it | 16:03 |
| noodles775 | Hi sinzui, would you have time for a small UI review? https://code.edge.launchpad.net/~michael.nelson/launchpad/509370-access-non-unique-ppa-name/+merge/19022 | 16:03 |
| jtv | sinzui: I didn't know pep8 said that... it's certainly not what we normally do. Would you be willing to take it out? | 16:03 |
| noodles775 | sinzui: it's not urgent, so I can wait for someone else if not. | 16:03 |
| noodles775 | mrevell: Do you mind going over the wording change on the above MP too? | 16:04 |
| * mrevell reads up | 16:04 | |
| mrevell | noodles775, would like to | 16:04 |
| mrevell | meaning, yes, I'll look now | 16:05 |
| noodles775 | mrevell: thanks. | 16:05 |
| jtv | sinzui: in the migrater, you're protecting the removal of the setup and teardown hooks against exceptions... is that specifically to guard against the case where they were never inserted? If so, better to specify the exception type (and make sure they're in the safe order) | 16:05 |
| sinzui | jtv: it does say that. one day I will replace my navel-lint script that I gave Launchpad with my pure python lint that I switched to last year | 16:05 |
| jtv | sinzui: what does say what? | 16:07 |
| sinzui | jtv: One of the setup/teardowns was written oddly in the code and that killed the migrater. I added the try/except to see it complete. so that I could manually fix the issue. I discovered that the code was migrated correctly, so I left the try/except there | 16:07 |
| sinzui | jtv: I would rather 1) remove the script, and 2) other teams complete the work we agreed they would do. I am just trying to finish their work | 16:08 |
| === deryck changed the topic of #launchpad-reviews to: on call: allenap, jtv, Edwin || reviewing: al-maisan, sinzui, abentley || queue [deryck] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| deryck | hi allenap, jtv, and EdwinGrubbs -- I put one on the queue if someone can spare the review. | 16:12 |
| jtv | sinzui, deryck: otp | 16:20 |
| jtv | sinzui: I don't suppose there's any chance of this messing up a migration, ever? | 16:25 |
| jtv | the try/except clause, I mean? | 16:25 |
| === allenap changed the topic of #launchpad-reviews to: on call: allenap, jtv, Edwin || reviewing: deryck, sinzui, abentley || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | jtv:I can remove it since I am done with it. I usually do that, but I think that is also a contributing factor why engineers are not using the tool. Only jml has committed fixes to the script besides myself | 16:26 |
| jtv | sinzui: no point in leaving it broken, I guess—especially now that what's left is the modules that weren't migrated easily early on | 16:27 |
| EdwinGrubbs | abentley: review sent | 16:27 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, jtv, Edwin || reviewing: al-maisan, sinzui, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | sinzui: so if this solves a real problem, leave it in. But a comment would be helpful either way. | 16:27 |
| jtv | on call: allenap, jtv, Edwin || reviewing: al-maisan, -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | 16:48 |
| jtv | allenap: did you take muharem's branch? | 16:48 |
| jtv | allenap: I just went over it as well, so if you're just starting, I can take it over from you | 16:49 |
| allenap | jtv: I did one of his branches recently. | 16:49 |
| jtv | allenap: the one about suspending jobs for disabled archives | 16:49 |
| allenap | jtv: That's the badger. | 16:49 |
| * jtv refreshes page | 16:49 | |
| jtv | oh right | 16:50 |
| jtv | I do wonder what the pre-imp said about re-enabling archives | 16:50 |
| jtv | I had a lot of trouble with that test function as well—the scope of the "not" in the name is ambiguous. AFAICS it would've been as easy, but clearer, to write to separate checks | 16:51 |
| allenap | jtv: Yeah... you should comment. I didn't think about that. | 16:52 |
| * jtv comments | 16:53 | |
| jtv | allenap: done | 17:00 |
| abentley | EdwinGrubbs, thanks for your review. | 17:03 |
| === jamalta_ is now known as jamalta | ||
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, Edwin || reviewing: sinzui, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | allenap: good night! | 17:38 |
| === jtv changed the topic of #launchpad-reviews to: on call: jtv, Edwin || reviewing: -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | jtv: Cheerio :) | 17:38 |
| === jtv changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | EdwinGrubbs: you're on your own | 17:58 |
| EdwinGrubbs | bye | 17:59 |
| === deryck is now known as deryck[lunch] | ||
| bac | sinzui: where is the branch you mentioned? even if it isn't ready to review i'd like to look at it. | 18:22 |
| sinzui | bac: indeed I was about to tell you to look at lp:~sinzui/launchpad/all-upstream-links . There is a test I need to update since beuno and I decided to present the table differently. | 18:28 |
| bac | sinzui: is 'status' too weasly as in 'Upstream information status" | 18:46 |
| sinzui | bac:umm...yes it is | 18:46 |
| bac | yeah, i thought so | 18:48 |
| sinzui | bac: This information is not necessarily about state. I think of these points as services, but I know that is not why we need to know them. This is really about the points where a project connects to different kinds of contributors | 18:48 |
| bac | what if were just labeled "Upstream Connections" | 18:49 |
| sinzui | bac: okay, I like that better than every other thought I have had.... | 18:50 |
| bac | just grasping at straws here | 18:50 |
| sinzui | bac: I merged this information into the project information portlet. Maybe we can test this portlet name by naming a new portlet on the project page. <Community|Contributor> connections? | 18:51 |
| bac | ok | 18:52 |
| sinzui | bac: think the launchpad projects you work with. Would owners complete this information if it were presented with this name? | 18:54 |
| sinzui | bac: is "Upstream contributor connections" a better name for your portlet? | 18:55 |
| bac | i like the layout you've done. but i think we need to make it easier to navigate to the action. | 18:56 |
| bac | if i wanted to set the bug tracker where is it done? | 18:57 |
| sinzui | bac: I think that is the next blueprint... | 18:58 |
| sinzui | we really need to get this blueprints's UI on edge for critique while we work on the upstream side... | 18:58 |
| bac | so links to the places to set the data will come later? | 18:59 |
| sinzui | bac: There are several issues, that are too much for this release: 1) change permissions so that you can set the information, and 2) change the forms so that the information can be set in context | 19:00 |
| bac | sinzui: ok. that makes sense. | 19:00 |
| sinzui | bac: Can you really set the series branch from a vocabulary? And as for the f&cking bug tracker. you need to go to the hidden page to register it, then find the field on the project to set it--we can solve this like the Create release link on series I hope | 19:01 |
| === deryck[lunch] is now known as deryck | ||
| bac | sinzui: do we want to macrotize the work you've done...or push the bool computations into a view mixin? | 19:08 |
| sinzui | bac: I think so. | 19:08 |
| bac | sinzui: which? both? | 19:08 |
| sinzui | bac: my branch introduces: <metal:yes-no define-macro="yes-no"> | 19:09 |
| bac | i'd say both. i'd rather have the login in a view | 19:09 |
| bac | yeah, the yes-no is cool | 19:09 |
| bac | but i don't want to repeat tal:define="bool not:series/translations_autoimport_mode/enumvalue:NO_IMPORT"> | 19:10 |
| bac | and the bug tracker logic is more complicated than you've shown b/c it has to take parent project group info into account | 19:10 |
| sinzui | If the tables I created work for the portlet, I would push them into either in a macro or an view that exposes the four points | 19:11 |
| bac | sinzui: ok, let me steal from your branch and see if it works. we can refactor afterwards | 19:12 |
| sinzui | bac: what? why can my project group have a bug tracker but not let me report a bug on the project group? | 19:12 |
| bac | sinzui: if firefox.bugtracker is None and official_malone is None but mozilla has a bug tracker then it will inherit mozilla's | 19:13 |
| bac | that's my understanding | 19:13 |
| sinzui | I did not know that, and that make me very upset that I cannot report a bug against mozilla | 19:14 |
| sinzui | bac: but given this, I favor a view that adapts a series or project that exposes the information. so that the template remains stupid | 19:15 |
| bac | i'm all for stupid-ass templates | 19:15 |
| bac | sinzui: but i think i've explained myself badly | 19:15 |
| bac | or even poorly | 19:15 |
| sinzui | bac: it should adapt the series because that is what is in the Packaging object | 19:16 |
| bac | you can report a bug against mozilla and firefox could have its own bug tracker but if it doesn't it uses the parent's | 19:16 |
| sinzui | bac: You cannot report a bug against launchpad-project...it defaults to blueprints. Most bugs reported in blueprints were from users who know launchpad is busted and they could not identify which componenet | 19:20 |
| bac | i didn't know that | 19:21 |
| * bac has never tried | 19:21 | |
| sinzui | project groups do get a working dupe finder though...much better than our retarded decision to ask users to report bugs against /launchpad. Most bugs reported there are dupes. At least the bugs reported against blueprints are not | 19:21 |
| sinzui | bac: I updated my tests to check for "Upstream Contributor Connections" in the table. Are you working on a view+template to adapt the series to to two list layout? | 19:58 |
| bac | sinzui: not yet. i was going to do some hard code experiments to see what how i can format the existing portlet | 19:59 |
| sinzui | bac: I was going to put my branch into review. I can instead extract the portable info to a view and template now | 20:00 |
| bac | sinzui: that would be good | 20:01 |
| sinzui | okay I will start | 20:01 |
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | bac: I just wanted to check if you had any concerns about my reply to your review since I had left some of the changes out due to complications. | 23:07 |
| bac | nope | 23:07 |
| bac | sorry i didn't comment | 23:07 |
| bac | EdwinGrubbs: i did mark it as 'approved' after your reply...which was a subtle way to signal consent | 23:07 |
| EdwinGrubbs | ok, thanks. I hadn't noticed that it was approved before, or I would have asked you earlier. | 23:08 |
| === jamalta is now known as jamalta-afk | ||
| sinzui | barry: land it | 23:14 |
| barry | sinzui: rock | 23:14 |
| sinzui | funny, that is almost the same as my review | 23:15 |
| barry | sinzui: :) | 23:15 |
| barry | sinzui: while i have you here, can you verify for me? we only store oauth tokens for Persons not Accounts, right? | 23:15 |
| sinzui | barry: it is on the Person | 23:17 |
| barry | sinzui: that's what i thought. what i don't know is whether we can limit reviews to folks who have lp accounts. they might only have logins to ubuntu.com | 23:17 |
| barry | if it's the latter, then the problem we have is that we need to verify who is rating an app, and i think without oauth we have no way to do that | 23:18 |
| sinzui | barry: since lp profiles are created on demand when someone logins to lp, it is kind of moot | 23:18 |
| barry | sinzui: hmm... so let's say they've never heard about lp, but they've bought something from the store or set up their u1 account... | 23:19 |
| sinzui | barry: PersonSet.enrsurePerson('ubuntu-one-user') will create the user when he uses lp | 23:19 |
| barry | sinzui: does that work if the first time they see lp is through an oauth request via launchpadlib? | 23:20 |
| sinzui | I wouldn't know. | 23:20 |
| barry | sinzui: okay, i'll try to hunt that down | 23:20 |
| sinzui | auth is a foundations/isd matter. I see from bugs too many parts of launchpad try to woek with an account...the look up for a person fails the moment the email is accessed, but by using ensurePerson(), the conversion happens without pain | 23:21 |
| barry | sinzui: it feels like we're really close to something usable. essentially we just want to use lp to verify who the person is and link their review to a login.u.com account | 23:21 |
| sinzui | barry: I think the crux of the issue is oauth. The steps are: | 23:23 |
| sinzui | 1) user submits a review to lp | 23:23 |
| sinzui | 2) lp gets user via ensurePerson() | 23:23 |
| sinzui | 3) lp asks user to authorise the script | 23:23 |
| barry | sinzui: yes, i think that's about it | 23:24 |
| sinzui | barry: you could use anonymous, and it may be possible to insert the user who is using the client | 23:24 |
| barry | sinzui: i don't follow | 23:24 |
| sinzui | I do not know how it is possible to allow an anonymous script to do an insert, but if it could happen, the script passes the review and with the user the client *claims* is using it | 23:25 |
| sinzui | I have never used anonymous access. | 23:26 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!