From 6c87fdc5624ea6fb2dd4e8ed940f799ef0e618b9 Mon Sep 17 00:00:00 2001
From: smcv <smcv@web>
Date: Wed, 10 Sep 2014 04:39:53 -0400
Subject: [PATCH] re-review

---
 doc/todo/calendar_autocreate.mdwn | 40 +++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/doc/todo/calendar_autocreate.mdwn b/doc/todo/calendar_autocreate.mdwn
index 02659d0d1..46cfea3bd 100644
--- a/doc/todo/calendar_autocreate.mdwn
+++ b/doc/todo/calendar_autocreate.mdwn
@@ -181,3 +181,43 @@ sub gencalendaryear {
 ---
 
 [[smcv]], can you please go on reviewing this?
+
+> I don't think I'm really the reviewer you want, since I don't have commit
+> access (as you might be able to tell from the number of pending branches
+> I have)... but nobody with commit access seems to be available to do
+> reviews at the moment, so I'm probably the best you're going to get.
+>
+>     +    0 0 * * * ikiwiki ~/ikiwiki.setup --refresh
+>
+> I think that should be `ikiwiki --setup ~/ikiwiki.setup`.
+>
+> The indentation of some of the new code in `IkiWiki/Plugin/calendar.pm`
+> is weird. Please use one hard tab (U+0009) per indent step: you seem
+> to have used a mixture of one hard tab per indent or two spaces
+> per indent, which looks bizarre for anyone whose tab size is not
+> 2 spaces.
+>
+>     +	return unless $config{calendar_autocreate};
+>
+> This is checked in `gencalendaryear` but not in `gencalendarmonth`.
+> Shouldn't `gencalendarmonth` do it too? Alternatively, do the check
+> in `scan`, which calls `gencalendarmonth` directly.
+>
+>     +		my $year  = $date[5] + 1900;
+>
+> You calculate this, but you don't seem to do anything with it?
+>
+>     +  if (not exists $changed{$params{year}}) {
+>     +    $changed{$params{year}} = ();
+>     +  }
+>     +  $changed{$params{year}}{$params{month}} = 1;
+>
+> $changed{$params{year}} is a scalar but `()` is a list. I think you want `{}`
+> (a scalar that is a reference to an empty anonymous hash).
+>
+> However, that whole `if` block can be omitted, and you can just use
+> `$changed{$params{year}}{$params{month}} = 1;`, because Perl will automatically
+> create $changed{$params{year}} as a reference to a hash (the term to look
+> up if you're curious is "autovivification").
+>
+> --[[smcv]]
-- 
2.45.0