[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/private-branches-tips into lp:launchpad

2014-01-06 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/private-branches-tips into lp:launchpad. Requested reviews: William Grant (wgrant) For more details, see: https://code.launchpad.net/~cprov/launchpad/private-branches-tips/+merge/200530 Refactoring IDistribution.getBranchTips() to deal

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/cric-api-take2 into lp:launchpad has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200550 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/cric-api-take2 into lp:launchpad with lp:~stevenk/launchpad/cric-model as a prerequisite. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/db-add-cric-take2/+merge/200560 -- https://code.launchpad.net

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/cric-api-take2 into lp:launchpad has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200563 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/cric-api-take2 into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200563 -- https://code.launchpad.net/~cprov

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad

2014-01-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~cprov/launchpad/db-add-cric-take2/+merge/200560 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/private-branches-tips into lp:launchpad

2014-01-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/private-branches-tips into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~cprov/launchpad/private-branches-tips/+merge/200530 --

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/extend-ensurepresent-timeout into lp:launchpad

2014-01-08 Thread Celso Providelo
Review: Approve Willian, As discussed on IRC, the hardcoded timeout scale (*5) is a little evil, but is well isolated on the ensurepresent() scope. As I said, withing the CI context, among jenkins and buildbot slaves (that are not that better in resilience), we could investigate a better

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bug-966837 into lp:launchpad

2014-01-09 Thread Celso Providelo
Review: Approve Willian, Thanks for the fix, the hanging PID files situation is unfortunate and to avoiding manual intervention (file removal) seems worth of a quick PID is alive check instead of a simple path check. Another, so called, unfortunate situation is the fact that we still have

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bug-1186349 into lp:launchpad

2014-01-13 Thread Celso Providelo
Review: Approve Willian, It ended up requiring a lot of changes to simply add batching support to a page. One would expect pre-existing (correct) support for that kind of operation in the DecoratedResultSet engine. I am glad you found time to add a more useful hook in the DecoratedResultSet.

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-14 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/cric-api-take2 into lp:launchpad. Requested reviews: William Grant (wgrant) Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/200563 -- https

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/branch-unscan-api into lp:launchpad

2014-01-14 Thread Celso Providelo
Review: Approve Thanks for the feature. Just minor remarks for your consideration: * What was the problem with simplejson on tests ? * assertIsNone(xxx) reads better than assertIs(None, xxx) -- https://code.launchpad.net/~wgrant/launchpad/branch-unscan-api/+merge/201704 Your team Launchpad

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/openid-provider-root-config-keys into lp:launchpad

2014-01-14 Thread Celso Providelo
Review: Approve William, Thanks for clarifying your plans on IRC and splitting the changes in small and readable chunks. -- https://code.launchpad.net/~wgrant/launchpad/openid-provider-root-config-keys/+merge/201545 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/openid-provider-root into lp:launchpad

2014-01-14 Thread Celso Providelo
Review: Approve William, Clear change, very good. Can you restore scripts/ppa-report.py permission in this change. If it is legitimate, please do it in another branch, possibly adjusting similar issues. -- https://code.launchpad.net/~wgrant/launchpad/openid-provider-root/+merge/201546 Your

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/openid-provider-root-cleanup into lp:launchpad

2014-01-14 Thread Celso Providelo
Review: Approve Looks good too! -- https://code.launchpad.net/~wgrant/launchpad/openid-provider-root-cleanup/+merge/201555 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad

2014-01-14 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/db-add-cric-take2 into lp:launchpad. Requested reviews: William Grant (wgrant) Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/db-add-cric-take2/+merge/200560

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/trusty-pls into lp:launchpad

2014-01-16 Thread Celso Providelo
Review: Approve Just confirmed the changes are functional in precise. Looks good to me! -- https://code.launchpad.net/~wgrant/launchpad/trusty-pls/+merge/202031 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list:

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-21 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/202478 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/cric-api-take2 into lp:launchpad

2014-01-21 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/cric-api-take2 into lp:launchpad. Requested reviews: Celso Providelo (cprov) For more details, see: https://code.launchpad.net/~cprov/launchpad/cric-api-take2/+merge/202478 Testfix -- https://code.launchpad.net/~cprov/launchpad/cric

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/faceted-breadcrumbs into lp:launchpad

2014-02-18 Thread Celso Providelo
Such a clever change! Now we have breadcrumb adapters tagged by facet names instead of vhosts. Perhaps we could use this opportunity to also rename the factory classes ThingVHostBreadcrumb to ThingFacetBreadcrumb just for the cleanness of terms. --

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/faceted-breadcrumbs into lp:launchpad

2014-02-18 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~wgrant/launchpad/faceted-breadcrumbs/+merge/206916 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/distroseries-translations-admin into lp:launchpad

2014-02-19 Thread Celso Providelo
Review: Approve Looks good, thank you for this. -- https://code.launchpad.net/~wgrant/launchpad/distroseries-translations-admin/+merge/207095 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/diff-navigator into lp:launchpad

2014-02-20 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/diff-navigator into lp:launchpad has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~cprov/launchpad/diff-navigator/+merge/205689 --

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/no-cscvs-svn into lp:launchpad

2014-02-24 Thread Celso Providelo
Review: Approve It is always good to have less code. As you stated on IRC, we still need subvertpy for bzr-svn, so the dependency remains in setup.py a CSCVS still needed as long as we have over 600 (!!) CVS active imports. The old subversion imports via CSCVS records will remain in DB for

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/diff-navigator into lp:launchpad

2014-02-24 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/diff-navigator into lp:launchpad. Requested reviews: William Grant (wgrant) For more details, see: https://code.launchpad.net/~cprov/launchpad/diff-navigator/+merge/205689 -- https://code.launchpad.net/~cprov/launchpad/diff-navigator

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/diff-navigator into lp:launchpad

2014-02-25 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/diff-navigator into lp:launchpad with lp:~cprov/launchpad/previewdiff-promotion as a prerequisite. Requested reviews: William Grant (wgrant): code For more details, see: https://code.launchpad.net/~cprov/launchpad/diff-navigator/+merge

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/previewdiff-promotion into lp:launchpad

2014-02-25 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/previewdiff-promotion into lp:launchpad. Requested reviews: William Grant (wgrant) For more details, see: https://code.launchpad.net/~cprov/launchpad/previewdiff-promotion/+merge/208187 Changes to promote PreviewDiff as 1st-class API

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bug-727445 into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~wgrant/launchpad/bug-727445/+merge/208377 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/kill-excessive-facets into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~wgrant/launchpad/kill-excessive-facets/+merge/208265 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/kill-facet-link-titles into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve Wow! That was a lot of code for little benefit. The enable_only property for ProjectGroup was clever! -- https://code.launchpad.net/~wgrant/launchpad/kill-facet-link-titles/+merge/208273 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/translator-url into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~wgrant/launchpad/translator-url/+merge/208262 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/mainsite-links into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve Cool! Fantastic use-case for the feature-flags framework. It brings a lot of safety for wide-changes like that. Well done! -- https://code.launchpad.net/~wgrant/launchpad/mainsite-links/+merge/208276 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/no-custom-xmlrpc-rooturl into lp:launchpad

2014-02-26 Thread Celso Providelo
Review: Approve Nice! I presume you will coordinate with webops to get those changes propagated to production configs at some point after it gets released. I am assuming (by the code) it won't break existing code with old-style configs because you have modified getRootURL() do operate

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/diff-navigator into lp:launchpad

2014-02-27 Thread Celso Providelo
I think I have addressed all the initial review issues. It is now rendering the diff navigator select on client-side and the content is not that bad for a initial release, although we could use something similar to fmt:displaydate leveraging Y.Intl Y.Date features. You have mentioned the

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/diff-navigator into lp:launchpad

2014-02-27 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/diff-navigator into lp:launchpad has been updated. Commit Message changed to: Support diff navigation in Merge Proposals. This is a preparation step towards supporting inline review comments. For more details, see:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/db-cve-2014 into lp:launchpad/db-devel

2014-03-03 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/db-cve-2014 into lp:launchpad/db-devel. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1287184 in Launchpad itself: Please support new CVE format https://bugs.launchpad.net/launchpad/+bug/1287184

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad

2014-03-06 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad. Requested reviews: William Grant (wgrant) For more details, see: https://code.launchpad.net/~cprov/launchpad/p3a-subscriptions/+merge/209813 -- https://code.launchpad.net/~cprov/launchpad/p3a

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad

2014-03-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad has been updated. Description changed to: Allow cancellation of archive subscriptions (P3A) via the API. This action can be performed by the target user, P3A owner, admins and commercial-admins. Implements

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad

2014-03-06 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/p3a-subscriptions into lp:launchpad has been updated. Commit Message changed to: Allow cancellation of archive subscriptions (P3A) via the API. For more details, see: https://code.launchpad.net/~cprov/launchpad/p3a-subscriptions/+merge/209813 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-api-token into lp:launchpad

2014-03-10 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/p3a-api-token into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1290543 in Launchpad itself: getArchiveSubscriptionURL(archive) should return the generated URL, not the user

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-diffpruner into lp:launchpad

2014-03-10 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-diffpruner into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-diffpruner/+merge/210304 Prevent periodic-cleanup of PreviewDiffs

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/pubdistro-bug-1290481 into lp:launchpad

2014-03-10 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/pubdistro-bug-1290481 into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1290481 in Launchpad itself: Gap between publishing custom uploads and signing them https

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/pubdistro-bug-1290481 into lp:launchpad

2014-03-10 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/pubdistro-bug-1290481 into lp:launchpad has been updated. Commit Message changed to: Suppress finalize.d parts (mirroring) running when publishing empty security suites. It prevents mirroring of unsigned custom uploads (signing is done in publish.d

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-api-token into lp:launchpad

2014-03-10 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/p3a-api-token into lp:launchpad has been updated. Status: Rejected = Needs review For more details, see: https://code.launchpad.net/~cprov/launchpad/p3a-api-token/+merge/210303 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-api-token into lp:launchpad

2014-03-10 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/p3a-api-token into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1290543 in Launchpad itself: getArchiveSubscriptionURL(archive) should return the generated URL, not the user

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-api-token into lp:launchpad

2014-03-10 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/p3a-api-token into lp:launchpad has been updated. Description changed to: Requiring a valid subscription in IPerson.getArchiveSubscriptionURL() before querying/creating authorisation tokens. For more details, see:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/p3a-api-token into lp:launchpad

2014-03-11 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/p3a-api-token into lp:launchpad has been updated. Commit Message changed to: Fix IPerson.getArchiveSubscriptionURL() to require a valid archive subscription before querying/creating an authorisation token. For more details, see:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/archive-subscription-mgmt into lp:launchpad

2014-03-17 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/archive-subscription-mgmt into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/archive-subscription-mgmt/+merge/211422 Granting lp.Edit

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ppa-copies-with-custom into lp:launchpad

2014-03-18 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ppa-copies-with-custom into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ppa-copies-with-custom/+merge/211620 Fixing permission

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-emails into lp:launchpad

2014-03-23 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-emails into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-emails/+merge/212326 Include review *inline* comments in the merge

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ui-stab-one into lp:launchpad

2014-03-31 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-ui-stab-one into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-ui-stab-one/+merge/213542 Moving 'diff-content update' code from

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ui-stab-two into lp:launchpad

2014-04-02 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-ui-stab-two into lp:launchpad with lp:~cprov/launchpad/ic-ui-stab-one as a prerequisite. Commit message: Create *scroller* links to navigate from review comments to their related inline-comments. Requested reviews: Launchpad code

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ui-stab-two into lp:launchpad

2014-04-03 Thread Celso Providelo
)) { 249 + // DiffNav was not properly initialized. 250 + return 251 + Missing semicolon. Added. [] -- Celso Providelo celso.provid...@canonical.com https://code.launchpad.net/~cprov/launchpad/ic-ui-stab-two/+merge/213950 Your team Launchpad code reviewers is subscribed to branch

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/726465 into lp:launchpad

2014-04-07 Thread Celso Providelo
Review: Approve The new docstring corresponds to the current code tests (even if branches have subscriptions, they can be deleted). -- https://code.launchpad.net/~cjohnston/launchpad/726465/+merge/214293 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/812225 into lp:launchpad

2014-04-07 Thread Celso Providelo
Review: Approve Chris, Thanks for re-wording this. It will possibly alleviate some confusing on bug-filing as suggested in the bug report, but IMHO it doesn't address the underlying issue as required by #1334. My only concern is that this small change calls people's attention to #1334 when

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad

2014-04-08 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-ui-prettify/+merge/214872 Styling diff inline-comments. Also

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/617209 into lp:launchpad

2014-04-09 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~cjohnston/launchpad/617209/+merge/213961 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/608478 into lp:launchpad

2014-04-09 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~cjohnston/launchpad/608478/+merge/213956 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/621471 into lp:launchpad

2014-04-09 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~cjohnston/launchpad/621471/+merge/214040 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/ic-view into lp:launchpad

2014-04-09 Thread Celso Providelo
Review: Approve Looks great, Chris! I am not particularly sure about the font line-height changes at this point, because they make inline comments too sparse. But that's more a personal taste issue than a opposition to land the changes. Soon we will get some time with UI experts and they

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/kill-bpb-is_new into lp:launchpad

2014-04-09 Thread Celso Providelo
Review: Approve Nice cleanup! There is a weird behaviour of makeBinaryPackageUpload() in the test that forces you to check properties of the *second* binary (when it should be only one). But that is unrelated to this change and surely manifest itself in many other tests, let's fix it later.

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-show-hide into lp:launchpad

2014-04-14 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-show-hide into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-show-hide/+merge/215691 Implements another auxiliary widget to show

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-show-hide into lp:launchpad

2014-04-14 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/ic-show-hide into lp:launchpad has been updated. Description changed to: Implements another auxiliary widget to show/hide rendered inline comments. Also fixes presentation order of draft and published inline comments. Drafts are always presented after

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-show-hide into lp:launchpad

2014-04-14 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/ic-show-hide into lp:launchpad has been updated. Description changed to: Implements another auxiliary widget to show/hide rendered inline comments. Fixes presentation order of draft and published inline comments. Drafts are always presented after

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/please-dont-generateListings into lp:launchpad

2014-04-15 Thread Celso Providelo
Review: Approve Great job! finalize.d is excellent for accommodating *specialties* like this. -- https://code.launchpad.net/~wgrant/launchpad/please-dont-generateListings/+merge/215925 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad

2014-04-16 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad has been updated. Status: Needs review = Rejected For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-ui-prettify/+merge/214872 --

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/bug-1295318 into lp:launchpad

2014-04-22 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/bug-1295318 into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1295318 in Launchpad itself: getPublishedSources launchpadlib query times out https://bugs.launchpad.net/launchpad

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/bug-1295318 into lp:launchpad

2014-04-23 Thread Celso Providelo
. That said, there are plans to redesign the package upload workflow (and queue) entirely and this particular aspect might change and those callsites will have to be revisited. 170 + def create_testing_ppa(self): This isn't a legal method name. Right, fixed. [] -- Celso Providelo

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/builderset-new-api into lp:launchpad

2014-04-23 Thread Celso Providelo
Review: Approve Looks good! It seems to be a good compromise before re-thinking the buildmaster component for the Cloud(tm). However, since we do not have a remove() builders should be created with caution, even by Administrators. --

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/blargh-userdata into lp:launchpad

2014-04-29 Thread Celso Providelo
Review: Approve Looks great! Thank you for being so diligent with cleanups. -- https://code.launchpad.net/~wgrant/launchpad/blargh-userdata/+merge/217531 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-ux-stab-1 into lp:launchpad

2014-05-07 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-ux-stab-1 into lp:launchpad. Commit message: Anonymous users can use DiffNav() to navigate across existing diffs and view published inline comments on public MPs Requested reviews: William Grant (wgrant): code For more details, see

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/fix-word-break into lp:launchpad

2014-05-12 Thread Celso Providelo
Review: Approve Thanks for working on this. I have only on minor comment about replacing L.insert(0,...) for a append(''); extend(...); append('') Once that's done we can land it. Inline comments: --- lib/lp/app/javascript/inlineedit/assets/skins/sam/editor-skin.css 2014-04-09 19:58:33

[Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/fix-word-break into lp:launchpad

2014-05-12 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cjohnston/launchpad/fix-word-break into lp:launchpad. Requested reviews: Celso Providelo (cprov) For more details, see: https://code.launchpad.net/~cjohnston/launchpad/fix-word-break/+merge/219285 - Set publish inline comments to true by default

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad

2014-05-13 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cprov/launchpad/ic-superseded-mps/+merge/219431 Suppressing inline comment

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad

2014-05-14 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad has been updated. Description changed to: Fixing several fallouts of the Review Inline Comments implementation by moving the few remaining view-implementation to JS. For more details, see:

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad

2014-05-14 Thread Celso Providelo
Most issues addressed. I've proposed to postpone the event-bubbling experiment (since the change would be in the link_scroller, not IC) and the pagetest-rewrite. Inline comments: --- lib/lp/app/javascript/comment.js 2014-04-04 05:23:48 + +++ lib/lp/app/javascript/comment.js 2014-05-14

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad

2014-05-14 Thread Celso Providelo
All set, thanks Inline comments: --- lib/lp/app/javascript/comment.js 2014-04-04 05:23:48 + +++ lib/lp/app/javascript/comment.js 2014-05-15 03:58:24 + @@ -308,8 +308,6 @@ initializer: function() { this.vote_input = Y.one('[id=field.vote]'); this.review_type

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad

2014-05-14 Thread Celso Providelo
The proposal to merge lp:~cprov/launchpad/ic-superseded-mps into lp:launchpad has been updated. Commit Message changed to: Fixing several fallouts of the Review Inline Comments implementation by moving the few remaining view-implementation to JS. For more details, see:

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/unicode-diff-comments into lp:launchpad

2014-05-15 Thread Celso Providelo
Review: Approve Thanks for fixing my mistakes :-) Looks good for landing. Inline comments: --- lib/lp/code/mail/codereviewcomment.py 2014-05-12 23:56:01 + +++ lib/lp/code/mail/codereviewcomment.py 2014-05-15 00:23:24 + @@ -174,13 +174,14 @@ result_lines = []

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/yay-sha512 into lp:launchpad

2014-05-19 Thread Celso Providelo
Inline comments: --- lib/lp/archivepublisher/model/ftparchive.py 2014-01-17 02:03:41 + dummy comment. +++ lib/lp/archivepublisher/model/ftparchive.py 2014-01-29 18:25:21 + @@ -72,7 +72,7 @@ DeLinkLimit 0; MaxContentsChange 12000; FileMode 0644; -}

[Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/yay-sha512 into lp:launchpad

2014-05-19 Thread Celso Providelo
The proposal to merge lp:~wgrant/launchpad/yay-sha512 into lp:launchpad has been updated. Status: Superseded = Rejected For more details, see: https://code.launchpad.net/~wgrant/launchpad/yay-sha512/+merge/203814 -- https://code.launchpad.net/~wgrant/launchpad/yay-sha512/+merge/203814 Your

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/publish-ic-link into lp:launchpad

2014-05-19 Thread Celso Providelo
Please address the issues pointed as inline comments and we can have another review-round. Inline comments: --- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-05-15 04:18:54 + +++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-05-19

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/publish-ic-link into lp:launchpad

2014-05-19 Thread Celso Providelo
Review: Needs Fixing -- https://code.launchpad.net/~cjohnston/launchpad/publish-ic-link/+merge/220103 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/publish-ic-link into lp:launchpad

2014-05-19 Thread Celso Providelo
Review: Needs Fixing Looks good, thanks Chris. I just think we should verify that tests (and feature) work without recreating the scroller on updates. There is no reason why it shouldn't since the 'end'ing code checks for draft(s) in runtime. Inline comments: ---

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/publish-ic-link into lp:launchpad

2014-05-19 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/publish-ic-link into lp:launchpad. Commit message: Creating a link_scroller to take users from the end of the diff up to the review comment form with proper focus. Requested reviews: Launchpad code reviewers (launchpad-reviewers

Re: [Launchpad-reviewers] [Merge] lp:~cprov/launchpad/publish-ic-link into lp:launchpad

2014-05-19 Thread Celso Providelo
Review: Approve Review performed in https://code.launchpad.net/~cjohnston/launchpad/publish-ic-link/+merge/220103 -- https://code.launchpad.net/~cprov/launchpad/publish-ic-link/+merge/220149 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/delete-branches-with-ics into lp:launchpad

2014-05-20 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/delete-branches-with-ics into lp:launchpad. Commit message: Allow the proper deletion of branches (and merge proposals) associated with inline comments. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/ic-js-cleanup into lp:launchpad

2014-05-20 Thread Celso Providelo
Review: Approve It looks great and I have no code remarks, everything was nicely implemented and/or clarified. Thanks for taking the opportunity to improve the original IC functions while working in a more flexible layout. We are already in a much better shape for implementing new feature and

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/ic-js-cleanup into lp:launchpad

2014-05-20 Thread Celso Providelo
Hints on the conflict resolution. Inline comments: --- lib/canonical/launchpad/icing/css/modifiers.css 2014-05-14 09:23:56 + +++ lib/canonical/launchpad/icing/css/modifiers.css 2014-05-21 01:55:47 + @@ -166,7 +166,8 @@ } pre.changelog, -table.diff, +table.diff

Re: [Launchpad-reviewers] [Merge] lp:~cjohnston/launchpad/diff-return-text into lp:launchpad

2014-05-26 Thread Celso Providelo
Review: Approve Chris, the change looks good and we can land it now and benefit from the wider audience feedback. Later on, when re-structuring the whole JS code, it looks like we should move rc_scroller creation from DiffNav() to the PublishDraft() domain, where it is updated. --

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/fix-draft-deletion into lp:launchpad

2014-05-29 Thread Celso Providelo
Review: Approve Nice one! and the no-tests-landing is only ok because we plan to refactor this piece of JS very soon. -- https://code.launchpad.net/~wgrant/launchpad/fix-draft-deletion/+merge/221362 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/non-js-link-regression into lp:launchpad

2014-05-29 Thread Celso Providelo
Review: Approve Uhm, It escaped my eyes in previous reviews ... Thanks for fixing it. -- https://code.launchpad.net/~wgrant/launchpad/non-js-link-regression/+merge/221395 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/branch-rewrite-http into lp:launchpad

2014-05-30 Thread Celso Providelo
Review: Approve William, The test runs the rewrite task as DBFuncLayer (which has security proxies) and checks it does the right thing for private branches. I guess that if we are sure 'use_web_security=True' recreates the same scenario when running the script, we are good to go. If we want

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/prf-bin into lp:launchpad

2014-06-03 Thread Celso Providelo
Review: Approve -- https://code.launchpad.net/~wgrant/launchpad/prf-bin/+merge/221889 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/b-m-integration-test into lp:launchpad

2014-06-09 Thread Celso Providelo
Review: Approve Nice test and very valuable asset just-before implementing the changes for the new reset protocol. LGTM! -- https://code.launchpad.net/~wgrant/launchpad/b-m-integration-test/+merge/222308 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bug-1083709 into lp:launchpad

2014-06-11 Thread Celso Providelo
Review: Approve Very sane, now isDriver() works for IHasDriver and IHasDrivers objects. Thanks for working on this. -- https://code.launchpad.net/~wgrant/launchpad/bug-1083709/+merge/222895 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bug-736005-begins into lp:launchpad

2014-06-13 Thread Celso Providelo
Review: Approve Let's crop this low-hanging fruit, very good! -- https://code.launchpad.net/~wgrant/launchpad/bug-736005-begins/+merge/223035 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list:

[Launchpad-reviewers] [Merge] lp:~cprov/launchpad/create-spec-webservice into lp:launchpad

2014-06-16 Thread Celso Providelo
Celso Providelo has proposed merging lp:~cprov/launchpad/create-spec-webservice into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1329424 in Launchpad itself: cannot create specification via API https://bugs.launchpad.net/launchpad/+bug

Re: [Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/builderset-current_build into lp:launchpad

2014-06-20 Thread Celso Providelo
Review: Approve Excellent! The API extension will be very useful in the scalingstack scenario and thanks to bringing more sanity to the builder/bpbuild/bq preloading area. -- https://code.launchpad.net/~wgrant/launchpad/builderset-current_build/+merge/223863 Your team Launchpad code reviewers

  1   2   >