I committed some whitespace, addressed some documentation lengths.

I'm curious of the couple date calls in ga_importer_cron() and ga_importer_get_data_for_node should be format_date()?

CommentFileSizeAuthor
more_polish.patch6.1 KBdstol

Comments

dstol’s picture

dstol
Primary language English
commented
Status: Active » Needs review
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Generally looks good to me - 

  drupal_set_message(st('Google Analytics Importer installed.'));

That's somewhat non-standard and in my opinion we shouldn't do it. If it made sense to give that success message during install then core would do it for us. I'd prefer removing those two lines.

dstol’s picture

dstol
Primary language English
commented

fine by me, what about format_date vs date in #1?

dstol’s picture

dstol
Primary language English
commented
Status: Needs review » Fixed

http://drupal.org/node/1032584

Left it as date for now.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.