review

1 files changed, 38 insertions, 0 deletions

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