156 + def get_template(name, from_template, default=False): 157 + filenames = get_preseed_filenames(node, name, release, default) 158 + filepath, content = get_preseed_template(filenames) 159 + if filepath is None: 160 + raise TemplateNotFoundError(name) 161 + return PreseedTemplate( 162 + content, name=filepath, get_template=get_template) 163 + 164 + return get_template(prefix, None, default=True)
Can you please add comments explaining that get_template is a Tempita hook and that you need to preserve the context for it via the closure since the hook is called out of scope. 227 + list(get_preseed_filenames(node, type, release, True))) When passing boolean arguments, it's much more useful to the reader to use the keyword argument here so something like: list(get_preseed_filenames(node, type, release, default=True))) is far more readable, especially if more bool params are added later. 236 + list(get_preseed_filenames(node, type, release, True))) Same here and in the new tests. 275 + def test_preseed_template_b64decode(self): 281 + def test_preseed_template_b64encode(self): Some comments on these, and on the 130 +class PreseedTemplate(tempita.Template): 131 + """A Tempita template specialised for preseed rendering.""" to explain why it's specialised would be very useful. I can see it's something to do with b64 stuff but it's hard to tell exactly what at a glance. -- https://code.launchpad.net/~rvb/maas/preseed-customisation/+merge/110861 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/preseed-customisation into lp:maas. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
