Noissue - Document the prefs module

lib/prefs.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 12 matching lines @@
 let defaults = Object.create(null);
 let overrides = Object.create(null);
 
 /**
  * Only for compatibility with core code. Please do not change!
  *
  * @type {boolean}
  */
 defaults.enabled = true;
 /**
- * The application version as set during intilization. Used to detect updates.

[kzar, 2015/04/16 12:11:14]

Nit: initialization

[kzar, 2015/04/16 13:32:34]

What about these comments?

[Sebastian Noack, 2015/04/16 13:47:06]

I didn't addressed them yet, or do you read "Done"? ;)

[kzar, 2015/04/16 13:48:55]

You quite commonly say "What about this comment" on my reviews should I not
reply to one. I assumed you would expect the same.

[Sebastian Noack, 2015/04/16 13:51:43]

Done.

[Sebastian Noack, 2015/04/16 13:51:43]

Only if you uploaded a new patch set, and I have reason to assume that you
overlooked those comments or try to cheat around addressing them. ;P

Done.

[kzar, 2015/04/16 13:54:50]

You had uploaded a new patch set though...

[Sebastian Noack, 2015/04/16 13:57:02]

... before you published your comments. ;)

+ * The application version as set during initialization. Used to detect updates.
  *
  * @type {string}
  */
 defaults.currentVersion = "";
 /**
  * Only for compatibility with core code. Please do not change!

[kzar, 2015/04/16 12:11:14]

It would be nice to add a little detail for all these ones that we ask not to be
changed. It might help me to grok the core code later for example.

[Sebastian Noack, 2015/04/16 12:52:00]

Those are already documented under https://adblockplus.org/en/preferences.
Duplicating the documentation or even linking to it here, would be misleading,
since those preferences don't have the same effect here, but changing them would
merely break the extension.

[kzar, 2015/04/16 13:32:34]

Fair enough but in that case how about adding a "@see
https://adblockplus.org/en/preferences" to them?

[Sebastian Noack, 2015/04/16 13:47:06]

I do for preferences that have an effect here. But as I said, those preferences
don't have the documented effect on Chrome, Opera and Safari, but would merely
break the extension. So there is no point in linking to the Firefox
documentation here. All that somebody working on this code needs to know is that
he must not touch those preferences.

  *
  * @type {string}
  */
 defaults.data_directory = "";
 /**
  * @see https://adblockplus.org/en/preferences#patternsbackups
  * @type {number}
  */
 defaults.patternsbackups = 5;
 /**
@@ skipping 78 matching lines @@
  * @type {boolean}
  */
 defaults.shouldShowBlockElementMenu = true;
 /**
  * Whether to collapse placeholders for blocked elements.
  *
  * @type {boolean}
  */
 defaults.hidePlaceholders = true;
 /**
- * Whether to show the first run page. This preference isn't
+ * Whether to suppress the first run page. This preference isn't
  * set by the extension but can be pre-configured externally.
  *
+ * @see https://adblockplus.org/development-builds/suppressing-the-first-run-page-on-chrome

[Sebastian Noack, 2015/04/18 14:36:27]

Note that this page isn't up yet, but will be at the time this change is going
to be pushed.

  * @type {boolean}
  */
 defaults.suppress_first_run_page = false;
 
 /**
   * @namespace
   * @static
   */
 let Prefs = exports.Prefs = {
   /**
    * Fired when the value of a preference changes.
    *
    * @event
    * @property {string} pref The name of the preference that changed.
    */
   onChanged: new ext._EventTarget(),
 
   /**
    * Fired when all preferences have been loaded. You must wait for
-   * this event before using preferences during extension intilization.

[kzar, 2015/04/16 12:11:14]

Nit: initialization

[Sebastian Noack, 2015/04/16 13:51:43]

Done.

+   * this event before using preferences during extension initialization.
    *
    * @event
    */
   onLoaded: new ext._EventTarget()
 };
 
 function keyToPref(key)
 {
   if (key.indexOf(keyPrefix) != 0)
     return null;
@@ skipping 119 matching lines @@
     });
   }
   else
   {
     managedLoaded = true;
     checkLoaded();
   }
 }
 
 init();