From: Joey Hess Date: Mon, 26 Jan 2009 19:13:52 +0000 (-0500) Subject: review X-Git-Tag: 3.03~23 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=96073d77af7530cd0a530fc71bd2ff076fa1f87d;p=ikiwiki.git review --- diff --git a/doc/plugins/contrib/po.mdwn b/doc/plugins/contrib/po.mdwn index af748a8d3..490cc2d6c 100644 --- a/doc/plugins/contrib/po.mdwn +++ b/doc/plugins/contrib/po.mdwn @@ -198,3 +198,41 @@ finish it at some point in the first quarter of 2009. --[[intrigeri]] > subsequent adjustments. > > --[[intrigeri]] + +> I've looked it over and updated my branch with some (untested) +> changes. +> +> Sorry, I'd forgotten about your cansave hook.. sorry for the duplicate +> work there. +> +> Reviewing the changes, mostly outside of `po.pm`, I have +> the following issues. +> +> * renamepage to renamelink change would break the ikiwiki +> 3.x API, which I've promised not to do, so needs to be avoided +> somehow. (Sorry, I guess I dropped the ball on not getting this +> API change in before cutting 3.0..) +> * I don't understand the parentlinks code change and need to figure it +> out. Can you explain what is going on there? +> * canrename's mix of positional and named parameters is way too +> ugly to get into an ikiwiki API. Use named parameters +> entirely. Also probably should just use named parameters +> for canremove. +> * `skeleton.pm.example`'s canrename needs fixing to use either +> the current or my suggested parameters. +> * I don't like the exporting of `%backlinks` and `$backlinks_calculated` +> (the latter is exported but not used). +> * What is this `IkiWiki::nicepagetitle` and why are you +> injecting it into that namespace when only your module uses it? +> Actually, I can't even find a caller of it in your module. +> * I'm very fearful of the `add_depends` in `postscan`. +> Does this make every page depend on every page that links +> to it? Won't this absurdly bloat the dependency pagespecs +> and slow everything down? And since nicepagetitle is given +> as the reason for doing it, and nicepagetitle isn't used, +> why do it? +> * The po4a Suggests should be versioned to the first version +> that can be used safely, and that version documented in +> `plugins/po.mdwn`. +> +> --[[Joey]]