Review: Needs Fixing code
Hi Colin,
Overall, I like this branch, and I'm happy for it to land (I'll just
have Robert hold you to your commitment to trim the LoC elsewhere ;)).
Just one question:
188 + # Really IBinaryPackagePublishingHistory, patched in
189 + # _schema_circular_imports.py.
190 + @operation_returns_entry(Interface)
191 + @operation_parameters(
192 + new_component=TextLine(title=u"The new component name."),
193 + new_section=TextLine(title=u"The new section name."),
194 + new_priority=TextLine(title=u"The new priority name."))
195 + @export_write_operation()
196 + @operation_for_version("devel")
197 + def changeOverride(new_component=None, new_section=None,
198 + new_priority=None):
I'm confused as to why you're using TextLine here for new_priority
(unless there's some pitfall in using copy_field() between different
interfaces; I don't know about that). You should be able to use
copy_field() here to copy the relevant field ( in this case I'm guessing
it'd be IBinaryPublishingHistory.priority), which would mean that you
don't have to do a lookup for the priority in name_priority_map later on
(line 297 of the diff)- lazr.restful would take care of that for you.
You can see an example of this in lib/lp/bugs/interfaces/bugtask.py:750.
--
https://code.launchpad.net/~cjwatson/launchpad/export-change-override/+merge/109549
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
