User talk:Legoktm/Memento

Jump to navigation Jump to search

I've been updating the code and committing. I haven't run the test suite yet.

Shall I update this page with my status so far?

Thanks again for the review.

Shawnmjones (talk) 02:04, 17 July 2014 (UTC)

Yes please! :) Legoktm (talk) 03:10, 17 July 2014 (UTC)

I've got a few questions so far.

  1. For the onImageBeforeProduceHTML code:
    • This code is experimental, and optional. It would be nice if it were enabled. I'm not sure how to solve the image problem for non-locally hosted files and would welcome any suggestions.
  2. For onArticleViewHeader code:
    • getTitle()->exists() vs. getTitle()->isKnown():
      • as noted in my comment, I was confused by the documentation
  3. For default configuration - I just made the values explicitly for the array to 1 through 15 rather than calculating it; not quite as flexible as I would like, but still a sensible default.
    • the $wgMementoExcludeNamespaces variable is supposed to be set, by default, to all of the non NS_MAIN namespaces; seeing as sites can configure their own namespaces, it was suggested by the wikitech-l folks to set this to array_diff( MWNamespace::getValidNamespaces(), MWNamespace::getContentNamespaces() ) by default
    • unfortunately, I cannot set this value using getValidNamespaces() at the entry point for the extension (Memento.php); the getValidNamespaces() blows up with "Unsupported operand types in /var/www/html/demo-devel/includes/Namespace.php on line 222"
  4. MementoResource->fetchMementoFromDatabase()
    • I tried to use $db->selectRow(), but it returned null, so I set it back to $db->select() and it worked; I was very confused
  5. MementoResource->fixTemplate()
    • I'm still not sure how to support the parser cache without disableCache()
    • I haven't been able to test it yet, so I'm not sure selectRow() will work, see above comment
  6. TimeGateResourceFrom302TimeNegotiation
    • I tried using OutputPage::redirect($url, 302), but it didn't redirect, so I went back to setting the headers; I was very confused

Again, thanks to your review, the code is much better than it was. Shawnmjones (talk) 03:27, 24 July 2014 (UTC)