launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

> 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.

Done.

> 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)))
> [...]

Good point, fixed.

> 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.

I've actually removed all the specialization of PreseedTemplate because it turns out that we don't need the b64 encoding utilities after all (I found that out while working on the follow-up branch).  I've kept the class though because I'm pretty sure we will need some specialization very soon (escaping probably).
-- 
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.