Bugzilla

Review of attachment 8889301 [details] [diff] [review]:
-----------------------------------------------------------------

You know how I love looking at this dead import code. I was waiting for the first patch to be refreshed, hence the delay. Anyway, in an effort to clear my review queue down to the essential, I've given this a quick glance. Whilst adding debug doesn't hurt, I'm a unsure whether we should add debug to code that is essentially not executed since these importers are pretty much dead, as I said on various occasions.

::: mailnews/import/applemail/src/nsAppleMailImport.cpp
@@ +469,5 @@
> +        DUMP_INITWITHPATH_PATH_IMPORT("nsAppleMailImportMail::FindMboxDirs", siblingMailboxDir,
> +                                      (char *) NS_ConvertUTF16toUTF8(siblingMailboxDirPath).get(),
> +                                      rv);
> +#endif
> +        continue;               // xxx: we may want to print a big warning dialog.

XXX:

::: mailnews/import/applemail/src/nsAppleMailImport.h
@@ +1,1 @@
> +

Extra blank line.

@@ +16,5 @@
>  // logging facilities
>  static mozilla::LazyLogModule APPLEMAILLOGMODULE("APPLEMAILIMPORTLOG");
>  
> +#if !defined(IMPORT_LOG0)
> +/* It is likely that these are defined in mailnews/import/src/ImportDebug.h already */

Hmm, can we give a definitive answer, "likely" is likely to confuse the reader.

::: mailnews/import/src/ImportDebug.h
@@ +27,5 @@
> +
> +#define DUMP_INITWITHFILE_PATH_IMPORT(func, p1, p2, result)                    \
> +  {                                                                     \
> +    nsAutoCString nativePath;                                           \
> +    IMPORT_LOG4("(debug) %s: %s->InitWithFile(%s) failed: rv = 0x%08x\n", func, #p1, #p2, result); \

The proper logging for "rv" is like this:
MOZ_LOG(DBLog, LogLevel::Info, ("error opening db %" PRIx32, static_cast<uint32_t>(rv)));

::: mailnews/import/winlivemail/WMDebugLog.h
@@ +9,5 @@
>  #include "mozilla/Logging.h"
>  static mozilla::LazyLogModule WMLOGMODULE("IMPORT");  // Logging module
>  
> +#if !defined(IMPORT_LOG0)
> +/* It is likely that these are defined in mailnews/import/src/ImportDebug.h already */

Same comment here.