On Tue, 2010-07-27 at 20:26 +0000, Māris Fogels wrote:
> Review: Approve code
> Hi William,
Thanks for the review.
> This is a very good change! At first I was caught thinking
> testJudgeAndDominateWithDDEBs() needed refactoring, but was pleasantly
> surprised to find that you fully tested both DDEB policies in the
> test_publishing module. I guess testJudgeAndDominate is just checking
> that the components are integrated correctly.
Right. I refactored the related code in earlier branches to make this
style of testing possible. I think it worked pretty well.
> The only other question is about _getCorrespondingDDEBPublications():
> can it use the ISlaveStore instead of the IMasterStore? Check
> lib/canonical/launchpad/doc/db-policy.txt for the difference.
This is the sort of thing where it's absolutely critical that we have
up-to-the-second data. Using the slave store would be dangerous.
Hi Māris,
On Tue, 2010-07-27 at 20:26 +0000, Māris Fogels wrote:
> Review: Approve code
> Hi William,
Thanks for the review.
> This is a very good change! At first I was caught thinking inateWithDDEBs( ) needed refactoring, but was pleasantly inate is just checking
> testJudgeAndDom
> surprised to find that you fully tested both DDEB policies in the
> test_publishing module. I guess testJudgeAndDom
> that the components are integrated correctly.
Right. I refactored the related code in earlier branches to make this
style of testing possible. I think it worked pretty well.
> The only other question is about _getCorrespondi ngDDEBPublicati ons(): launchpad/ doc/db- policy. txt for the difference.
> can it use the ISlaveStore instead of the IMasterStore? Check
> lib/canonical/
This is the sort of thing where it's absolutely critical that we have
up-to-the-second data. Using the slave store would be dangerous.
> Otherwise this looks good to me: r=mars
Thanks!