Code review comment for lp://staging/~wgrant/launchpad/unuse-bugtask-markers

Revision history for this message
Curtis Hovey (sinzui) wrote :

I think this branch is good to land, but I have a few questions I hope to you can answer.

I see the milestone controls have a sensible name now, but I do not see how this was accomplished. I assume all is good since tests did not break.
- ... name='1.0_firefox.milestone')
+ ... name='firefox_1.0.milestone')

I think getMilestoneTarget() is wrong, and it has been wrong since mid 2006. Milestones names are owned by the pillar (since 2006), though their direct parent as of 2009 is the series. I expect the rules for productseries and distroseries to be identical, yet they are not. A the milestone target for a productseries is a product. Why is a distroseries a milestone target? And why is a distroseries the target for a sourcepackage?
+ if IProduct.providedBy(bug_target):
+ target = milestone_context.product
+ elif IProductSeries.providedBy(bug_target):
+ target = milestone_context.productseries.product
+ elif (IDistribution.providedBy(bug_target) or
+ IDistributionSourcePackage.providedBy(bug_target)):
+ target = milestone_context.distribution
+ elif (IDistroSeries.providedBy(bug_target) or
+ ISourcePackage.providedBy(bug_target)):
+ target = milestone_context.distroseries
You may not know the answer to this since this change preserves the existing behaviour. My question does not block your landing. This may relate to https://bugs.launchpad.net/launchpad/+bug/423692

review: Needs Information (code)

« Back to merge proposal