X-Git-Url: https://sipb.mit.edu/gitweb.cgi/ikiwiki.git/blobdiff_plain/aed4e03b0c850d48722d5f72cbf7bd6a2a052a12..062a915d7b5e4a6fcbfcca243d02a6aa9c4222ee:/doc/todo/calendar_autocreate.mdwn diff --git a/doc/todo/calendar_autocreate.mdwn b/doc/todo/calendar_autocreate.mdwn index 02659d0d1..371079d68 100644 --- a/doc/todo/calendar_autocreate.mdwn +++ b/doc/todo/calendar_autocreate.mdwn @@ -181,3 +181,58 @@ 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 --refresh` +> +> > [[Corrected|https://github.com/paternal/ikiwiki/commit/213dad76d47bab9db8e44d6e20c8371960375e77]] +> +> 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. +> +> > [[Corrected|https://github.com/paternal/ikiwiki/commit/1d97160dae775c31e166d9886472dacdd773d571]] +> +> + 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. +> +> > Once again, [[you are right|https://github.com/paternal/ikiwiki/commit/473bcbe7a42a4168cab82ed12185817248de045f]] +> +> + my $year = $date[5] + 1900; +> +> You calculate this, but you don't seem to do anything with it? +> +> > [[Corrected|https://github.com/paternal/ikiwiki/commit/d0b34951240317642543351ec62f98d3d8df8c0f]] +> +> + if (not exists $changed{$params{year}}) { +> + $changed{$params{year}} = (); +> + } +> + $changed{$params{year}}{$params{month}} = 1; +> +> `$changed{$params{year}}` is a scalar (you can tell because it starts with the +> `$` sigil) 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 an empty hash if necessary, +> in order to put the pair `$params{month} => 1` in it (the term to look +> up if you're curious is "autovivification"). +> +> > [[Corrected|https://github.com/paternal/ikiwiki/commit/d0b34951240317642543351ec62f98d3d8df8c0f]] +> +> --[[smcv]] +> +> > Thank you for your review. +> > --[[Louis|spalax]]