| === maxb_ is now known as maxb | ||
| thumper | EdwinGrubbs: https://code.launchpad.net/~thumper/launchpad/branch-api-expose-package-bits/+merge/19550 ? | 00:23 |
|---|---|---|
| thumper | EdwinGrubbs: very trivial | 00:23 |
| === jamalta is now known as jamalta-afk | ||
| mwhudson | thumper: feel free to review https://code.edge.launchpad.net/~mwhudson/launchpad/unicod-branch-names-bug-449528/+merge/19572 later if you like :-) | 03:19 |
| noodles775 | on call: noodles775 || reviewing: - || queue [adiroiban(bug-509252-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | 09:36 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue [adiroiban(bug-509252-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adiroiban(bug-509252-take-2) || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | noodles775, I have one with 2500 removed lines; can you take it? | 11:15 |
| noodles775 | salgado: pop it in the queue :), it sounds *fab* (-: | 11:16 |
| === salgado changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adiroiban(bug-509252-take-2) || queue [salgado] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | adiroiban: hi! thanks for all the cleanups that you included in your branch! | 11:18 |
| adiroiban | noodles775: hi. | 11:19 |
| adiroiban | everything is ok? | 11:19 |
| adiroiban | I should have done those changed one month ago, but by then I didn't know to much about LP | 11:20 |
| noodles775 | :), I was keen to know more of the background for removing the security check from the model code? | 11:20 |
| adiroiban | basicaly, I was cleaning some mess generated be me | 11:20 |
| noodles775 | Usually it needs to be both on the model and the view as view checks won't be relevant for API calls? | 11:21 |
| noodles775 | s/to be/to be used | 11:21 |
| adiroiban | talking with henninge, and I think there was also an email on lp-dev ml, | 11:22 |
| * noodles775 reads | 11:22 | |
| adiroiban | we decided not to include check_permission code in the model | 11:22 |
| henninge | noodles775: the problem is that check_permission does not take a user parameter and assumes the request user. You don't have that in the model. | 11:24 |
| henninge | the request | 11:24 |
| adiroiban | henninge: maybe we can add a new check_permission_with_user helper | 11:25 |
| henninge | adiroiban: no, we just need to add a user parameter to check_permission. Somebody suggested that on the ML, too. | 11:25 |
| adiroiban | :) | 11:26 |
| adiroiban | or that :) | 11:26 |
| adiroiban | noodles775: even in the code that is now in trunk there is no security check in the model | 11:36 |
| adiroiban | and they are done in 4 other places in the view | 11:37 |
| adiroiban | and the model.distroseries.checkTranslationViewable() is only called from the view | 11:38 |
| adiroiban | there is no other place in the model to call it | 11:38 |
| noodles775 | henninge, adiroiban: So if I'm understanding the email thread correctly, this branch is going *against* what was recommended isn't it? | 11:39 |
| noodles775 | adiroiban: yeah, I'm just trying to find where that security adapter is used. | 11:40 |
| henninge | noodles775: sorry, which email thread? | 11:40 |
| noodles775 | henninge: RFD: Overhauling the Launchpad authorization adapters | 11:41 |
| adiroiban | noodles775: I don't think it is against... as there is no check_permission in the model, or why do you think it is agains the recomandations? | 11:41 |
| noodles775 | AFAICS, the security check should go on the model, and be called by both the security adapter and the view? | 11:41 |
| noodles775 | adiroiban: if someone comes along later and exposes this via the API? | 11:42 |
| henninge | noodles775: yes, that is the current recommendation. | 11:42 |
| adiroiban | henninge, noodles775 , then we have to extend check_permission | 11:46 |
| adiroiban | because the current code from the model | 11:46 |
| adiroiban | is only doing half of the security checks | 11:46 |
| noodles775 | adiroiban: if you take a look at lib/lp/registry/interfaces/person.py:addMember() | 11:47 |
| noodles775 | you'll see it's got a reviewer attribute, which is set to the request user when this is accessed via the API. | 11:48 |
| noodles775 | Could adding a similar param to your model code check work similarly? | 11:48 |
| noodles775 | Or would that be dependent on check_permission having the extra param... hrm. | 11:49 |
| adiroiban | noodles775: no, because in my case, the security is done using classes from security.py | 11:49 |
| adiroiban | I am not sure that addind security related check in the model is a good idea. There must be a good reason why in zope we have the securityproxy, and the security checks are not done in the model layer | 11:51 |
| noodles775 | adiroiban: did you see BjornT's reply in the email? | 11:52 |
| adiroiban | noodles775: this one: https://lists.launchpad.net/launchpad-dev/msg02063.html ? | 11:54 |
| noodles775 | no, from the more recent thread above... hang on. | 11:56 |
| noodles775 | adiroiban: https://lists.launchpad.net/launchpad-dev/msg02504.html | 11:56 |
| adiroiban | noodles775: no. I'm reading the thread now | 11:56 |
| noodles775 | Specifically: We already have this. Have the check in model code, and have the security adapter ask the model. | 11:57 |
| noodles775 | OK. | 11:57 |
| adiroiban | noodles775: thanks. then I will have to extend check_permission and then come back to this branch | 12:04 |
| noodles775 | adiroiban: I'm not sure that you will need to (based on BjornT's reply)... | 12:05 |
| noodles775 | adiroiban: if you need to include the current user in the permission check, can't you: | 12:06 |
| noodles775 | 1. add an optional requestor param to the model security check, and then, | 12:06 |
| noodles775 | 2. Call this directly from the view, and | 12:06 |
| adiroiban | noodles775: this will not solve the current problem, as I need to use check_permissions | 12:07 |
| adiroiban | in the model | 12:07 |
| adiroiban | and check_permission does not accept an user param | 12:07 |
| noodles775 | adiroiban: ok, I didn't yet see the need to call check_permission from the model code... where is that call? | 12:08 |
| adiroiban | and like Barry Warsaw said, adding more security checks in the model is not a good idea for the long term | 12:08 |
| adiroiban | noodles775: in my branch, look at check_distroseries_translations_viewable from browser_helper.py | 12:09 |
| noodles775 | Sure, it's something the security infrastructure should manage, but in cases like these, until it does, moving them to the view has issues too. | 12:10 |
| * noodles775 checks | 12:10 | |
| adiroiban | if you look in the diff, you will see that checkTranslationsViewable() was allwasy called toghere with check_permission | 12:12 |
| adiroiban | together | 12:12 |
| adiroiban | and when changing check_permission in one place, it is easy to forget about the other place it is called together with checkTranslationsViewable | 12:13 |
| noodles775 | ? | 12:13 |
| noodles775 | adiroiban: yeah, so unless I've missed something, all you need to do is update AdminDistroSeriesLanguage.checkAuthenticated() to additionally call distroseries.checkTranslationsViewable()? | 12:13 |
| adiroiban | yes... but then distroseries.checkTranslationsViewable() will have to call AdminDistributionTranslations.checkAuthenticated | 12:15 |
| noodles775 | why? if all callsites use check_permission? (sorry if I've missed something obvious) | 12:15 |
| noodles775 | (ie. ignore my point 2 above, you'd use the check_permission call in the view as well, as is done currently) | 12:18 |
| adiroiban | from my point of view checkTranslationsViewable() is design for an exception | 12:19 |
| adiroiban | not for the rule | 12:19 |
| adiroiban | and all the „normal” rules are in security.py | 12:19 |
| adiroiban | this is why I was expecting to see the exception based on the „normal rules” | 12:19 |
| adiroiban | and not the „normal rules” based on the exception code | 12:19 |
| adiroiban | please let me know if I did not make myself understood | 12:20 |
| noodles775 | I hadn't thought about it in terms of what is based on what, but rather, how can the code be organised so that the check is in the one place to cover both the view, and other modes of access (such as the API). | 12:22 |
| noodles775 | s/view/browser view | 12:23 |
| adiroiban | the new check_distroseries_translations_viewable is called from the URL traversal code for each object | 12:27 |
| adiroiban | so it should also protect the API... or not ? | 12:27 |
| noodles775 | adiroiban: I hadn't seen the context of that, so yes (I think) it would, but couldn't/shouldn't it use check_permission? | 12:32 |
| adiroiban | it does use check_permission | 12:33 |
| noodles775 | (assuming the above changes.) | 12:33 |
| noodles775 | No, I mean the other way around, it should call the standard check_permission(), which would call the model's extra permission check. | 12:33 |
| noodles775 | gar, s/check_permission()/check_permission() directly | 12:34 |
| adiroiban | noodles775: Then I will have to move the logic from browser_helpers.py to security.py | 12:37 |
| adiroiban | is that what you are saying? | 12:37 |
| noodles775 | noodles775: no, as per the email, the logic would stay on the model (where it currently is), but be called by the security check in security.py. | 12:40 |
| noodles775 | adiroiban: ^^ | 12:40 |
| adiroiban | :) | 12:40 |
| adiroiban | but I can not use check_permission() from the model | 12:40 |
| * noodles775 thinks it's way past his lunch break - brain is doing crazy things ;) | 12:40 | |
| adiroiban | no hurry | 12:41 |
| adiroiban | we can continue after lunch | 12:41 |
| noodles775 | adiroiban: OK, that's the thing that I need clarified... I'm not currently seeing why you need to call check_permission from the model code. | 12:42 |
| * noodles775 goes to lunch | 12:42 | |
| adiroiban | enjoy your meal :) | 12:42 |
| noodles775 | Ta! | 12:42 |
| === bigjools is now known as bigjools-lunch | ||
| noodles775 | So adiroiban, why would you need to call check_permission from the model code? | 13:34 |
| adiroiban | because launchpad admins are allowed to see series with hidden translations | 13:34 |
| === mrevell is now known as mrevell-lunch | ||
| adiroiban | while for other the translations should be hidden | 13:35 |
| adiroiban | even if we move that method/function in the model | 13:39 |
| adiroiban | it will not be called from the model | 13:39 |
| adiroiban | or is URL traversal part of the model? | 13:39 |
| noodles775 | adiroiban: yes, but the view would be calling check_permission() directly, which would check exactly that (as it currently does) and then additionally call the IDistroSeries.checkTranslationsViewable() where necessary (ie. when they are not an admin). | 13:40 |
| noodles775 | And the URL traversal would also call check_permission() | 13:40 |
| adiroiban | noodles775: ok. I think I got it | 13:40 |
| adiroiban | :) | 13:40 |
| adiroiban | let me move the code and then show you the diff | 13:41 |
| noodles775 | OK, I'll summarise that on the MP and mark it as needs fixing... you can then attach the diff to your reply, thanks! | 13:41 |
| adiroiban | noodles775: so you want to replace all calls for check_distroseries_translations_viewable from the view | 13:41 |
| adiroiban | with just check_permission | 13:41 |
| noodles775 | Yes, assuming that your check_permission will also call IDistroSeries.checkTranslationsViewable(). | 13:42 |
| adiroiban | noodles775: it should :) | 13:42 |
| adiroiban | thanks! | 13:42 |
| noodles775 | np! | 13:43 |
| adiroiban | noodles775: there is still one detail | 13:45 |
| noodles775 | yep? | 13:45 |
| adiroiban | Danilo told me that the security check was not done in the security.py | 13:45 |
| adiroiban | so that the users will see a proper message | 13:45 |
| adiroiban | not „accees denied” | 13:45 |
| noodles775 | Right... hrm... | 13:45 |
| adiroiban | my check_distroseries_translations_viewable is doing 2 checks | 13:45 |
| adiroiban | one for launchpad.view | 13:46 |
| adiroiban | and one for launchpad.TranslatonsAdmin | 13:46 |
| noodles775 | adiroiban: where is that currently? All I can see are difirent exception strings in checkTranslationsViewable()... which are not permission related? | 13:47 |
| noodles775 | s/difirent/different ugh. | 13:48 |
| adiroiban | in the current code from trunk, the model.distroseries.checkTranslationsViewable is not doing any security checks | 13:48 |
| adiroiban | but if you search for the places where it is called | 13:49 |
| adiroiban | you will see there is a security check before each call | 13:49 |
| * noodles775 looks | 13:49 | |
| adiroiban | and there is also some delegation from translations.browser.distroseries.checkTranslationsViewable | 13:50 |
| adiroiban | which is doing some security checks and the it call registry.model.distroseries.checkTranslationsViewable | 13:50 |
| === salgado changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adiroiban(bug-509252-take-2) || queue [salgado*2] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | adiroiban: so is this an accurate summary: The view code needs to call the model code's check directly as it depends on exceptions being raised for non-admins, to give an appropriate message? | 13:57 |
| noodles775 | sorry salgado :/ | 13:57 |
| salgado | noodles775, ? | 13:58 |
| adiroiban | noodles775: not sure if is „model code's check” | 13:59 |
| noodles775 | salgado: that I still haven't gotten to your first review. | 13:59 |
| salgado | oh, no worries | 13:59 |
| adiroiban | the view needs to choose between checking for launchpad.View or launchpad.TranslationsAdmin | 13:59 |
| adiroiban | and this decision is based on the state of the distroseries | 13:59 |
| adiroiban | so yes. the view needs to call/read something from the model | 14:01 |
| adiroiban | in order to take the correct decission regarding what permission and error message to display | 14:01 |
| noodles775 | Right, so simply: if the user isn't a translations admin and translations are hidden, return a meaningful error. | 14:01 |
| adiroiban | yes... a meaningful error based on the series status | 14:02 |
| noodles775 | Yep... | 14:02 |
| noodles775 | OK, I think you were right to move most of checkTranslationsViewable() to the view, as most of it is not a security check, but error generation. Only the first 3 lines should be part of the security. | 14:03 |
| * noodles775 thinks a bit more. | 14:04 | |
| adiroiban | noodles775: the first 3 lines are in fact http://paste.ubuntu.com/379087/ | 14:06 |
| adiroiban | i guess | 14:06 |
| noodles775 | adiroiban: yes, exactly. | 14:06 |
| adiroiban | the probles is with this conditional security check | 14:07 |
| adiroiban | problem | 14:07 |
| adiroiban | it was the same issue that triggered Henning's email in December https://lists.launchpad.net/launchpad-dev/msg02061.html | 14:10 |
| noodles775 | adiroiban: http://pastebin.ubuntu.com/379097/ | 14:13 |
| noodles775 | if they have admin, they will see it, if they have view, they will see it, if they don't have view permission (because the hidden check would be in the security.py check for launchpad.View), they get the relevant error? | 14:14 |
| noodles775 | Ah, this then hits the problem that someone with Admin won't necessarily have View? | 14:15 |
| noodles775 | But that's easy to fix in security.py (by being explicit). | 14:15 |
| === mrevell-lunch is now known as mrevell | ||
| adiroiban | looking | 14:17 |
| noodles775 | So we're really missing a DistroSeriesLanguageView class in security.py I think? | 14:17 |
| adiroiban | noodles775: DistroSeriesLanguageAdminTranslations should call DistroSeriesAdminTranslation, which should call DistributionAdminTranslations | 14:18 |
| noodles775 | adiroiban: what's your skype id... might be quicker to discuss this :) | 14:20 |
| adiroiban | and you code is also in DistroSeriesNavigation | 14:20 |
| adiroiban | and your code is also in DistroSeriesNavigation | 14:20 |
| adiroiban | and it should be called from the translation URL traversal for DistroSeries and SourcePackage | 14:20 |
| adiroiban | hm... | 14:21 |
| adiroiban | let me install skype first :) | 14:21 |
| noodles775 | yes, it's an example just to separate the security from the error msg. | 14:21 |
| adiroiban | the id should be adiroiban | 14:21 |
| === mup_ is now known as mup | ||
| === bigjools-lunch is now known as bigjools | ||
| abentley | noodles775, could you please review https://code.edge.launchpad.net/~abentley/launchpad/restricted-diffs/+merge/19607 ? | 14:39 |
| noodles775 | abentley: please add it to the queue, if I don't get to it, someone else will. | 14:39 |
| === abentley changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adiroiban(bug-509252-take-2) || queue [salgado*2, abentley] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | noodles775: please allow me to install and make skype working on my computer, and also have lunch and will come back in about 30 minutes | 14:40 |
| noodles775 | adiroiban: np, I'll update the MP for the moment, enjoy your lunch! | 14:41 |
| === salgado is now known as salgado-lunch | ||
| noodles775 | henninge: can you please take a look at my comment on adiroiban's MP at: https://code.edge.launchpad.net/~adiroiban/launchpad/bug-509252-take-2/+merge/19484 | 14:51 |
| noodles775 | and let me know if it's sane? | 14:52 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [salgado, abentley] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | noodles775, got another one for your queue... is that too much? https://code.launchpad.net/~jtv/launchpad/bug-507681/+merge/19531 | 14:55 |
| noodles775 | jtv: yeah, I probably won't get to it (as I'll need to come back to an earlier review too), but put it on and we'll see :) | 14:55 |
| jtv | noodles775: will do | 14:56 |
| === jtv changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [salgado, abentley, jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | noodles775: back. skype test was working, skypeid adiroiban | 15:02 |
| noodles775 | salgado-lunch: when you're back, are there further branches coming? If I run a local server, I can't login... | 15:04 |
| noodles775 | adiroiban: great... one tic. | 15:04 |
| === matsubara is now known as matsubara-lunch | ||
| === bigjools changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [salgado, abentley, jtv, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | abentley: everything's stuck on 507681 now... can I get you to review it? | 15:39 |
| jtv | abentley: this mp: https://code.launchpad.net/~jtv/launchpad/bug-507681/+merge/19531 | 15:40 |
| abentley | jtv, is there a reason you want me specifically? Otherwise, I'd rather leave it to the OCR. | 15:41 |
| jtv | abentley: one of the wellington gang would be nice. Plus, OCR is backlogged, and this way you get to unblock your own branch-url branch. :-) | 15:42 |
| * jtv watches shouting mobs outside... must be a big football match on | 15:43 | |
| jtv | here comes the police... | 15:43 |
| jtv | ...and the riot police vehicle. Sport at its best! | 15:43 |
| bigjools | jtv: it didn't happen without pictures | 15:44 |
| jtv | damn | 15:44 |
| jtv | hang on | 15:44 |
| abentley | jtv, I'll see. I've got a lot going on right now. | 15:44 |
| jtv | bigjools: the action seems to have shifted out of range. Thank you, Erwin Rommel, for popularizing the meeting engagement. | 15:51 |
| jtv | abentley: I'll shop around a bit more. | 15:51 |
| bigjools | does Godwin's law apply here already? | 15:51 |
| jtv | bigjools: it doesn't work that way. | 15:51 |
| jtv | bigjools: but since we're talking, can I get you to review a buildfarm branch that we have several other branches blocked on? | 15:52 |
| bigjools | jtv: how big? | 15:52 |
| jtv | bigjools: see for yourself... https://code.launchpad.net/~jtv/launchpad/bug-507681/+merge/19531 | 15:53 |
| bigjools | jtv: might be able to look a bit later | 15:53 |
| bigjools | in an hour or so | 15:54 |
| jtv | bigjools: I would be indebted to the tune of several good beers | 15:54 |
| abentley | jtv, the diff has conflicts. | 15:54 |
| * bigjools will hold you to that at the next epic | 15:54 | |
| jtv | bigjools: let's hope it's in Belgium. | 15:54 |
| bigjools | ah you're going to uds? | 15:54 |
| jtv | abentley: thanks for pointing that out! | 15:54 |
| jtv | bigjools: no plans, no | 15:55 |
| bigjools | oh, beers... belgium.... | 15:55 |
| bigjools | I prefer ale from England ;) | 15:55 |
| === matsubara-lunch is now known as matsubara | ||
| === salgado-lunch is now known as salgado | ||
| salgado | noodles775, what happens when you try to login? | 15:56 |
| jtv | bigjools: I only recently had the chance to learn about proper ales | 15:56 |
| jtv | fixing the conflicts now... that's what I get for updating copyright dates. | 15:56 |
| noodles775 | salgado: DiscoveryFailure: HTTP Response status from identity URL host is not 200. Got status 503<br /> | 15:56 |
| jtv | ah, sirens now | 15:56 |
| salgado | noodles775, when people review one of these branches I always forget to mention they need to add testopenid.dev to /etc/hosts | 15:57 |
| salgado | sorry | 15:57 |
| salgado | noodles775, btw, flacoste has just approved that branch of mine | 15:57 |
| noodles775 | salgado: great. | 15:57 |
| abentley | jtv, with pipelines, the recommended way to merge is to merge into the first pipe, then run 'bzr pump'. | 15:57 |
| * noodles775 checks the size of abentley's branch. | 15:58 | |
| jtv | abentley: just what I was doing... I'm getting used to pipelines already | 15:58 |
| noodles775 | Nice. | 15:58 |
| === sinzui1 is now known as sinzui | ||
| abentley | jtv, cool. Then you can check out pump --from-submit next time, which automates it even further. | 16:01 |
| flacoste | salgado: i wanted to share the pleasure of remoing 2k lines! | 16:01 |
| jtv | abentley: oh! That's nice. | 16:02 |
| === noodles775 changed the topic of #launchpad-reviews to: noodles775 || reviewing: abentley || queue [jtv, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | flacoste, it felt good indeed. :) | 16:04 |
| henninge | noodles775: I will take care of Adi's branch. | 16:13 |
| noodles775 | henninge: thanks. | 16:15 |
| noodles775 | abentley: your branch looks great. The only question I have is whether there's really a need to include each segment of the url in the doctest: http://code.launchpad.dev/~person-name.../product-name.../branch.../+merge/.../+preview-diff/+files/preview.diff | 16:20 |
| abentley | noodles775, I would be fine with just showing "/+preview-diff/+files/preview.diff". Would you prefer that? | 16:21 |
| noodles775 | abentley: yeah (not that I care much either way, it'd just mean it was within 78chars :)) | 16:22 |
| jtv | kfogel, care to review a wording change you suggested? https://code.launchpad.net/~jtv/launchpad/bug-299008/+merge/19628 | 16:22 |
| kfogel | jtv: looking (but I'm not officially a reviewer) | 16:23 |
| jtv | kfogel: I don't think anybody would object in this case | 16:23 |
| kfogel | jtv: I'll just note that in my review so it's all above board, np. | 16:24 |
| jtv | kfogel: thanks | 16:24 |
| kfogel | jtv: heh, nice note | 16:25 |
| jtv | :) | 16:25 |
| kfogel | jtv: approved | 16:27 |
| jtv | kfogel: sheishei | 16:27 |
| jtv | kfogel: "䄟䄟"? | 16:28 |
| kfogel | jtv: :-) modern romanization: "xie xie"; older romanization system: "hsieh hsieh"; actual pronounciation "shuh-yea shuh-yea" :-) | 16:28 |
| kfogel | jtv: hunh, my xchat isn't displaying that right, unfortunately | 16:28 |
| jtv | kfogel: China Airlines, apparently, is not the most precise of Mandarin schools. | 16:28 |
| jtv | kfogel: wrong character set... never mind | 16:29 |
| kfogel | jtv: odd, because their academic reputation is excelelnt | 16:29 |
| jtv | gotta be better than their flight safety reputation :) | 16:29 |
| jtv | (never had any trouble myself, but istrm hearing something about that) | 16:30 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [jtv, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | thanks noodles775, enjoy your evening! | 16:40 |
| noodles775 | jtv: was that sarcasm? I didn't get to your branch ;) You too! | 16:41 |
| jtv | noodles775: no sarcasm—at least I'm at the front of the queue now, when there was a fair backlog ahead of me earlier | 16:42 |
| === deryck is now known as deryck[lunch] | ||
| gary_poster | uh. nobody on call. | 17:24 |
| leonardr | rockstar: are you available to review a multiversion branch? | 17:31 |
| rockstar | leonardr, yes. | 17:31 |
| leonardr | gary: if it's something i can take, i can review while my own branch is in review | 17:31 |
| leonardr | rockstar: ok, on the way | 17:31 |
| rockstar | gary_poster, I am OCR today, but had a personal errand this morning. | 17:31 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: leonardr || queue [jtv, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gary_poster | leonardr, rockstar: ok thank you. I got it taken care of I think. | 17:32 |
| === bigjools changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: leonardr || queue [jtv, bigjools*2] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === henninge is now known as henninge-afk | ||
| === gary_poster is now known as gary-lunch | ||
| leonardr | rockstar: https://code.edge.launchpad.net/~leonardr/lazr.restful/launchpad-integration/+merge/19639/ | 17:58 |
| === deryck[lunch] is now known as deryck | ||
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: bigjools || queue [jtv, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary-lunch is now known as gary_poster | ||
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: bigjools || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| * rockstar cooks through the backlog of reviews | 19:26 | |
| * abentley reminds rockstar of the UI review | 19:27 | |
| rockstar | abentley, yup, I know. | 19:27 |
| rockstar | abentley, ui=rockstar for you | 19:47 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | rockstar, thanks! | 19:49 |
| abentley | rockstar, what do you think about the two shades of yellow? | 19:50 |
| rockstar | abentley, well, I'm not sure if it really matters. | 19:53 |
| rockstar | abentley, maybe the diff message should be blue, indicating "informational" but yellow also seems to indicate "stale" so I'm a little conflicted. | 19:53 |
| rockstar | I didn't think it was a big enough deal to even worry about. | 19:53 |
| rockstar | If someone complains, then we can re-evaluate. | 19:54 |
| abentley | rockstar, I didn't actually get that "yellow means stale". I thought it meant "note". | 19:54 |
| rockstar | abentley, I think it means "caution" but when I was looking at the comments, I almost felt like it was like old yellowed paper. I don't know. | 19:55 |
| abentley | rockstar, Anyhow, we might want to look at settling on one shade of yellow. | 19:55 |
| rockstar | abentley, yeah, but I don't think it's a huge issue, so don't go out of your way for it. | 19:56 |
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | bac: can you look at the incremental diff I added to https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-99395-linking-sourcepackages-to-projects/+merge/19429 | 20:49 |
| * bac looks | 20:49 | |
| bac | looks good EdwinGrubbs | 20:51 |
| EdwinGrubbs | thanks | 20:52 |
| === EdwinGrubbs is now known as Edwin-lunch | ||
| === matsubara is now known as matsubara-afk | ||
| === sinzui changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | rockstar: do you have time for a short branch: https://code.launchpad.net/~sinzui/launchpad/packaging-timeout-bug-523886/+merge/19660 | 21:53 |
| rockstar | sinzui, for you, sure. | 21:59 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === Edwin-lunch is now known as EdwinGrubbs | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!