Review: Approve code
39 + @property
40 + def show_information_type_in_ui(self):
41 + feature_flag_base = 'disclosure.show_information_type_in_ui.enabled'
42 + if IBug.providedBy(self.context):
43 + pass
44 + elif IBranch.providedBy(self.context):
45 + feature_flag_base = feature_flag_base.replace('ui', 'branch_ui')
46 + else:
47 + raise NotImplemented()
48 + return bool(getFeatureFlag(feature_flag_base))
Since InformationTypePortlet isn't used as its own view (always as a mixin for
an edit form class), this horror can probably die or be replaced with "raise
NotImplementedError()". It should also possibly be renamed to
InformationTypePortletMixin, and the module name should be informationtype, not
information_type.
60 + if (
61 + self.context.information_type == InformationType.USERDATA and
62 + self.show_userdata_as_private):
Why is that extra newline there?
if (self.context.information_type == InformationType.USERDATA
and self.show_userdata_as_private):
71 + if (
72 + self.context.information_type == InformationType.USERDATA and
73 + self.show_userdata_as_private):
Likewise.
107 def initialize(self):
108 super(BugView, self).initialize()
These can be deleted, as the method looks empty apart from that super() call.
433 +class TestBranchPrivacyPortlet(TestCaseWithFactory):
That's a bit odd. Is there really nowhere else that tests the portlet?
485 + This branch contains <strong id="information-type"
tal:content="view/information_type"></strong> information
486 + <div id='information-type-description' style='padding-top: 5px'
tal:content="view/information_type_description"></div>
The nbsp before a new div doesn't strike me as particularly useful. Do you know
why it's there?
--
https://code.launchpad.net/~stevenk/launchpad/branch-information_type-ui/+merge/107909
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
