=== mwhudson_ is now known as mwhudson
=== daniloff is now known as danilos
=== 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
[11:40] <al-maisan> hello allenap, I have a nice branch for you :)
[11:40] <al-maisan> https://code.edge.launchpad.net/~al-maisan/launchpad/ppa-visibility-514824/+merge/20530
[11:47] <allenap> al-maisan: Cool, I'll do that now.
=== allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:47] <al-maisan> thanks!
=== matsubara-afk is now known as matsubara
[12:05] <allenap> al-maisan: In xx-distribution-packages.txt, the disabled PPA is created, and it's absence from the PPA list is the test. Can you add a comment along the lines of "The disabled PPA is not shown in the page". Or, better, demonstrate that it is shown, disable it, and demonstrate that it is no longer shown.
[12:06] <al-maisan> allenap: I'll add the comment if you don't mind. Our test suite already takes too much time to run :)
[12:07] <allenap> al-maisan: Okay :)
[12:08] <allenap> al-maisan: Also, ViewSourcePackagePublishingHistory could be changed to just inherit from ViewArchive, with a custom __init__() that calls the superclass with obj.archive.
[12:08] <al-maisan> allenap: good point .. I'll look into that.
[12:09] <allenap> al-maisan: ViewArchive.checkAuthenticated() already checks for user.is_admin, so both ViewSourcePackagePublishingHistory.checkAuthenticated() and ViewSourcePackagePublishingHistory.checkUnauthenticated() both defer entirely to ViewArchive.
[12:10] <al-maisan> allenap: right.
[12:10] <allenap> al-maisan: You don't need to change that though; the composition is quite understandable.
[12:10] <allenap> al-maisan: I have to go and feed children, but I'll be back in <1h. Other than those observations, it all looks good :)
[12:11] <al-maisan> allenap: take care of the children and thanks for the review!
[12:20] <adeuring> allenap: could you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-511240-bug-heat-include-duplicate-bugs/+merge/20533 ? a real bargain: only 80 lines
=== adeuring changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[13:05] <bigjools> al-maisan: I have a security fix, can I usurp the queue please :)
[13:05] <bigjools> err allenap, sorry
[13:13] <allenap> adeuring: Sure.
[13:13] <allenap> bigjools: Sure.
=== allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [bigjools, adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevell is now known as mrevell-lunch
[13:15] <allenap> Hey bigjools, where's your mp?
[13:15] <allenap> T'ain't in t'queue.
[13:15] <bigjools> allenap: I'm making a new one, the branch was for devel and the MP was for prod-devel so the diff was screwed
[13:16] <allenap> bigjools: Okeley dokely.
[13:18]  * bigjools fights bazaar merging
[13:52] <leonardr> allenap, may i join the queue? (https://code.edge.launchpad.net/~leonardr/lazr.restful/refactor-tag-request-with-version/+merge/20547)
[13:52] <allenap> leonardr: Sure, go for it :)
=== leonardr changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [bigjools, adeuring, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:23] <al-maisan> hello allenap, here are the changes we discussed prior to the lunch break: http://pastebin.ubuntu.com/387623/
[14:25] <allenap> al-maisan: They look good. Push them and I'll add my r=me to the mp.
[14:25] <al-maisan> allenap: will do, thanks!
[14:26] <al-maisan> changes pushed
[14:41] <danilos> adiroiban, btw, first things first: bug 525325 definitely doesn't belong in LP Foundations; I'd say it's either registry or translations, but that's a very minor point :)
[14:41] <mup> Bug #525325: Export more ILanguage attributes in API <Launchpad Foundations:In Progress by adiroiban> <https://launchpad.net/bugs/525325>
[14:42] <adiroiban> danilos: ok. I asked on lp-dev about the code from services/worlddata and I was told it is part of foundations
[14:47] <danilos> adiroiban, hum, ok; I'd say nobody really knows what is it part of, but LP Translations team mostly maintains it (which is why I never bothered moving browser and other code out of lp.translations)
=== allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: adeuring || queue [leonardr] || 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 || reviewing: - || queue [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:35] <abentley> allenap, could you please review https://code.edge.launchpad.net/~abentley/launchpad/qa-ready/+merge/20563 ?
[15:35] <danilos> adiroiban, hey, your branch is looking great, though I have a few questions :)
[15:36] <allenap> abentley: Sure, please add it to the queue. I won't get to it for a couple of hours because I have leonardr's branch to do and some groceries to get (my wife is too ill, still, to do that).
[15:36] <danilos> adiroiban, is there any reason not to use "only visible languages" for the default /languages call? and then provide an extra method that can return a subset of or all languages?
=== abentley changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [leonardr, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:36] <abentley> allenap, thanks.
[15:36] <allenap> leonardr: Actually, I'm not going to get to your branch either for an hour or so, due to grocery shopping. If it's urgent you might want to find another reviewer.
[15:37] <allenap> Sorry.
[15:37] <leonardr> allenap, not too urgent
[15:37] <allenap> Cool, I'll do both of those when I'm back.
[15:38] <adiroiban> danilos: the reason was to avoid exporting another `reallyGetAllLanguages` method
[15:39] <adiroiban> other than that, there is no reason. I can do that :)
[15:39] <danilos> adiroiban, well, I'd probably make it something like "getHiddenLanguages" or something; I am not sure myself what's the better pattern, so I'd like to discuss it before we settle on either way
[15:41] <adiroiban> danilos: then we can export only visible languages by default
[15:41] <adiroiban> and have a getAll operation that will get all languages
[15:41] <danilos> adiroiban, right, so how about a getVisibleLanguages as the default and getAllLanguages as another method?
[15:42] <adiroiban> I am not sure about the usage of getHiddenLangauges, but I guess it will always be used togheter with getVisibleLanguages
[15:42] <adiroiban> so getAll langauges can avoid a second call
[15:42] <danilos> adiroiban, though, will we ever have someone using API and not wanting hidden languages? for that matter, will we ever have anyone wanting to get all languages in the first place?
[15:44] <adiroiban> danilos: hidden languages can be used in Ubuntu to find our imported translations that diverge from the `main` language
[15:44] <adiroiban> other than that, I think most users will only care about visible languages
[15:45] <adiroiban> danilos: thus, I think we can just export all visible languages by default
[15:45] <adiroiban> and if someone ask
[15:45] <adiroiban> we can add an operation to export all languages (including hidden one)
[15:46] <adiroiban> in the current lazr.restful implementation the default operation has no name, and it is available via +languages
[15:47] <adiroiban> to get all langauges, you will need to explicitly call an operation +languages?ws.op=getAll
[15:47] <danilos> adiroiban, right, I'd tend to agree
[15:51] <adiroiban> danilos: ok. then I will change the code to export only visible languages
[15:51] <adiroiban> other issues?
[15:57] <danilos> adiroiban, I'm still going through it, if any, it's going to be only minor issues
[15:57] <danilos> adiroiban, stylistic and such
=== salgado is now known as salgado-lunch
[16:11] <noodles775> Hi allenap, no probs. if you won't get to it, but I'll just pop this one on the queue: https://code.edge.launchpad.net/~michael.nelson/launchpad/522517-multiple-subscriptions-displayed/+merge/20567
=== noodles775 changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [leonardr, abentley, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-lunch
[16:15] <bigjools> allenap: replied to your review
[16:16] <bigjools> allenap: the branch page is showing a diff with stuff missing though :/
[16:17] <bigjools> and now it's there after I pushed a new revision.  Weird.
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: - || queue [leonardr, abentley, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:41] <EdwinGrubbs> leonardr, abentley, noodles775: do any of you have urgent branches?
[16:41] <abentley> EdwinGrubbs, no.
[16:41] <leonardr> edwingrubbs, no
[16:41] <noodles775> EdwinGrubbs: no.
[16:43] <EdwinGrubbs> abentley: I'll take yours, since it looks like allenap said he would take leonardr's.
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: abentley || queue [leonardr, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:45] <abentley> EdwinGrubbs, cool.
[17:07] <leonardr> allenap, Edwin, i've got another branch for the queue
[17:07] <leonardr> https://code.edge.launchpad.net/~leonardr/lazr.restful/make-test-class-public/+merge/20574
=== leonardr changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: abentley || queue [leonardr, noodles775, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[17:08] <EdwinGrubbs> abentley: since I'm not really experienced in this part of the code, did you have a pre-impl call?
[17:08] <allenap> leonardr: I don't think I'll get to that, but I am about to start your first branch.
[17:08] <abentley> EdwinGrubbs, no.
[17:09] <EdwinGrubbs> abentley: how do I go about testing it? I know how to create a mirrored branch, but do I need to run a cronscript to do the mirroring?
[17:10] <abentley> EdwinGrubbs, that's not what we mean by mirror in this case.
=== salgado-lunch is now known as salgado
[17:10] <abentley> Your mirrors are your local copy of stable and your local copy of db-stable.
[17:12] <EdwinGrubbs> abentley: ok, it's making a lot more sense now. I'll try running it.
=== matsubara-lunch is now known as matsubara
=== allenap changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: abentley || queue [noodles775, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[17:45] <EdwinGrubbs> abentley: I just want to confirm that this is the intended behavior:
[17:45] <EdwinGrubbs> Mirror file:///home/egrubbs/canonical/lp-branches/db-stable/ is out of date.
[17:45] <EdwinGrubbs> Deployed on staging: False
[17:45] <EdwinGrubbs> abentley: when I updated the local mirror of db-stable, it then said True.
[17:45] <abentley> EdwinGrubbs, that sounds reasonable, if your local copy of db-stable is out of date.
[17:46] <abentley> Updating your mirror of db-stable should not change the status, though.
[17:47] <EdwinGrubbs> abentley: is that because it does graph.is_ancestor() against the local_branch instead of the remote one?
[17:47] <abentley> EdwinGrubbs, I assume it is, but that
[17:48] <abentley> is strange, because I wil have just updated the local branch in that case.
[17:48] <abentley> You weren't running qa-ready against your db-stable branch itself, were you?
[17:48] <EdwinGrubbs> abentley: no.
[17:49] <abentley> That is baffling.
[17:49] <EdwinGrubbs> abentley: according to your cover letter, "It has no effect on
[17:49] <EdwinGrubbs> the branch, only on what revisions are accessible in the branch's repository."
[17:50] <abentley> EdwinGrubbs, correct.
[17:50] <EdwinGrubbs> abentley: so would that explain why graph.is_ancestor() can't find it in local_branch.last_revision()?
[17:51] <abentley> EdwinGrubbs, hold up.
[17:51] <abentley> It appears to be because it does graph.is_ancestor against the local *mirror*, not the local branch.
[17:53] <abentley> Graph traversal is a repository operation.
[17:53] <abentley> So updating the repository of the local mirror should be sufficient.
[17:54] <EdwinGrubbs> abentley: are you going to be able to fix that easily?
[17:54] <abentley> So pull in your mirror of db-stable should have no impact on graph traversal.
[17:55] <abentley> EdwinGrubbs, I suspect that diagnosing the problem is going to be the most effort.
[17:56] <abentley> At this point, I still don't see how the behaviour you're reporting is possible.
[17:57] <abentley> EdwinGrubbs, could you verify that the revno of your db-stable mirror matches the revno of db-stable itself?
[18:01] <EdwinGrubbs> abentley: I had pulled it before, and they are both at rev 9072 now.
[18:01] <abentley> EdwinGrubbs, do you know what revno it was on before you pulled it?
[18:02] <EdwinGrubbs> abentley: this is interesting. If I uncommit the changes to my local db-stable branch and then run qa-ready, "Deployed on staging" is still True.
[18:02] <EdwinGrubbs> abentley: before I pulled it, it was several months old.
[18:02] <danilos> adiroiban, btw, it seems my review didn't get through via email; I've pasted it directly on the MP: only a few small bits left, it's a really great branch! thanks again :)
[18:02] <abentley> EdwinGrubbs, when you uncommitted, you uncommitted back to where it was before?
[18:04] <EdwinGrubbs> abentley: no, I just uncommited till I got to the last revision where stable is merged into db-stable. "bzr missing" shows that the branch I'm running it on has one extra revision.
=== jelmer_ is now known as jelmer
[18:06] <abentley> EdwinGrubbs, I'm trying to reproduce the issue locally.
=== danilos is now known as daniloff
[18:44] <leonardr> edwin: another super small branch for the queue
[18:44] <leonardr> https://code.edge.launchpad.net/~leonardr/launchpadlib/multiversion-fix/+merge/20585
=== leonardr changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: abentley || queue [noodles775, leonardr, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[18:53] <abentley> EdwinGrubbs, I was unable to reproduce the issue locally.  With a fresh branch of db-stable that was 50 revisions out of date, it still correctly determined the status of a branch that was deployed this morning.
[18:56] <EdwinGrubbs> abentley: I think that it is finding the revisions in the repository if they already are there, but the update of the repository during the script either doesn't update the repository correctly, or the script is ignoring the new data since the object were opened before that was added.
[18:57] <abentley> EdwinGrubbs, that's why I used a fresh branch that doesn't have those revisions.
[18:57] <EdwinGrubbs> abentley: but isn't that branch using a shared repository?
[18:57] <abentley> EdwinGrubbs, no, I created it as a standalone branch so that I could get an accurate test.
[18:58] <EdwinGrubbs> abentley: hmmm, that's weird
[18:59] <abentley> EdwinGrubbs, it may have used the revisions from the shared repository via the local_branch, so I'm running another test.
=== gary_poster is now known as gary-lunch
[19:31] <abentley> EdwinGrubbs, my second test also doesn't reproduce the problem.  I used a fresh copy of db-stable and the branch in question, and neither had the revision where the branch was merged.
[19:32] <EdwinGrubbs> abentley: ok, we'll assume it is a fluke, and wait for it to happen again. r=me
[19:32] <abentley> EdwinGrubbs, thanks.
=== EdwinGrubbs is now known as Edwin-lunch
=== jamalta-afk is now known as jamalta
=== gary-lunch is now known as gary_poster
[20:38] <leonardr> edwin, i'm leaving for the day. feel free to drop my branches from the queue if you find anything confusing
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
[21:34] <EdwinGrubbs> leonardr: how do I run the tests.
[21:34] <leonardr> EdwinGribbs: which branch?
[21:35] <leonardr> python bootstrap.py; bin/buildout; bin/test should work
[21:41] <EdwinGrubbs> trying it now
[21:43] <sinzui> EdwinGrubbs: ping
=== jamalta is now known as jamalta-afk