Review: Needs Fixing code 44 === modified file 'lib/lp/bugs/browser/tests/test_bugs.py'
It might be worth adding a test for the behaviour when the sharing policy is PUBLIC. 388 === modified file 'lib/lp/bugs/model/bug.py' 389 --- lib/lp/bugs/model/bug.py 2012-10-04 14:22:59 +0000 390 +++ lib/lp/bugs/model/bug.py 2012-10-07 23:54:23 +0000 391 @@ -2629,7 +2629,7 @@ 392 # XXX: ElliotMurphy 2007-06-14: If we ever allow filing private 393 # non-security bugs, this test might be simplified to checking 394 # params.private. 395 - if (IProduct.providedBy(params.target) and params.target.private_bugs 396 + if (IProduct.providedBy(params.target) 397 and params.target.bug_sharing_policy is None 398 and params.information_type not in SECURITY_INFORMATION_TYPES): 399 # Subscribe the bug supervisor to all bugs, private_bugs was a top-level conjunct, so the block could never fire unless it was set. By removing private_bugs we're treating it as if it was false everywhere, so this whole block should be deleted. 408 -class TestBugPrivateAndSecurityRelatedUpdatesMixin: 409 +class TestBugPrivateAndSecurityRelatedUpdatesMixin(TestCaseWithFactory): This is no longer a mixin, so please rename the class. 448 === modified file 'lib/lp/bugs/scripts/bugimport.py' 449 --- lib/lp/bugs/scripts/bugimport.py 2012-06-29 08:40:05 +0000 450 +++ lib/lp/bugs/scripts/bugimport.py 2012-10-07 23:54:23 +0000 451 @@ -295,9 +295,6 @@ 452 453 private = get_value(bugnode, 'private') == 'True' 454 security_related = get_value(bugnode, 'security_related') == 'True' 455 - # If the product has private_bugs, we force private to True. 456 - if self.product.private_bugs: 457 - private = True 458 information_type = convert_to_information_type( 459 private, security_related) Do we want to add an assertion in the importer to reject imports into a product with a policy other than PUBLIC? 461 @@ -323,11 +320,6 @@ 462 bug.linkMessage(msg) 463 self.createAttachments(bug, msg, commentnode) 464 465 - # Security bugs must be created private, so set it correctly. 466 - if not self.product.private_bugs: 467 - information_type = convert_to_information_type( 468 - private, security_related) 469 - bug.transitionToInformationType(information_type, owner) This block should simply be enabled, not removed. We'd ideally also create it with the correct information_type, now that we have that capability. 516 def test_createBug_proprietary_subscribers(self): This test will probably fail due to your change on line 395. Once that block is removed, this test no longer has value and can be removed. 650 - field_names = [ 651 - "project_reviewed", 652 - "license_approved", 653 - "active", 654 - "private_bugs", 655 - "reviewer_whiteboard", 656 - ] 657 + field_names = ["project_reviewed", "license_approved", "active", 658 + "reviewer_whiteboard"] I think we prefer the old style, but we certainly don't prefer the new style without an initial newline. 795 """See `IDistribution.`""" orly -- https://code.launchpad.net/~stevenk/launchpad/death-to-private_bugs/+merge/128398 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
