In addition to the review comments below, I also fixed some typos here and there. They are attached in the diff to this email, but I will also push them to my branch at lp:~hingo/drizzle/drizzle-docs71 so you can just continue merging from there.
henrik On Sun, Oct 30, 2011 at 7:55 PM, Henrik Ingo <[email protected]> wrote: > Hi Daniel > > That's a respectable piece of documentation you've written! And a bit > like I've discovered too, as part of writing end user documentation > you ended up writing 2 completely new modules too, so that there is a > nice end user experience worth documenting. > > I tried to read through everything at > ~daniel-nichter/drizzle/7.1-docs. I provide my review as comments > here, since I don't want to just change stuff without you being aware > of it, and in some cases it's really just an opinion you may or may > not disagree with. Here it goes... > > > docs.drizzle.org > > So I see this is now committed to trunk, yet docs.drizzle.org still shows some > old documentation. What's the point of this? To me it seems some people really > went through a lot of trouble to keep Drizzle's capabilities a secret > from the wider > public: First they avoid documenting any of it, then they make sure > that if others > document something, it's not published... > > The documentation committed to trunk should automatically be published on > docs.drizzle.org > > > docs/dirhtml/contributing/more_ways/index.html > - link to actual donation page! > https://co.clickandpledge.com/advanced/default.aspx?wid=46722 > > /docs/dirhtml/installing/requirements/index.html > - this page smells old. Ubuntu 10.04 is tested but not 11.*? > - The list of dependencies is suspiciously short. I've added > dependency on libv8, you'd need to configure --without-js-plugin to > avoid hitting an error without it installed. > > > /docs/dirhtml/installing/ubuntu/index.html > - When debs are made available, needs distinguishing between standard > Ubuntu packages and newest from Drizzle PPA. > > docs/dirhtml/installing/redhat/index.html > - Compiling from source is wrong, use yum build-depends (copy from README) > > docs/dirhtml/configuration/options/index.html > > I would put also this: > plugin-remove=auth_all > plugin-add=auth_file > in the plugin specific config file (conf.d/auth_file.conf). That way > if I remove > the conf file, plugin is not loaded. Is there a reason why this shouldn't be > used, recommended or does it even work? In any case, the example could use > some > truly drizzled lever options too, to make the point clearer. > > Btw, this needs hacking the build system, but we should develop a system where > each plugin exports a default configuration into > /etc/drizzle/available-plugins/hello_world.conf > and to enable a plugin you symlink it to /etc/drizzle/conf.d/hello_world.conf > and then edit as needed. (This implies the plugin-add is part of its own > configuration file, not top level.) > > > > Many places, but for example docs/dirhtml/configuration/options/index.html > - I disagree with using > .. code-block:: bash > for an interactive session such as using "drizzle". It makes words like > "for" > stand out highlighted. Only the first line in these code blocks is bash. > > docs/dirhtml/configuration/index.html > - This will require some work, but we should script something that creates a > page with a table of all options and variables, their types and default and > allowed values. (Similar in spirit to > http://dev.mysql.com/doc/refman/5.5/en/mysqld-option-tables.html) > This is very Google friendly. > > docs/dirhtml/administration/drizzled/index.html > - I was cc'd off-list on an email from Patrick and Kent which says by default > drizzle now writes to syslog. (Kent was channeling Brian.) > Why these emails are not shared with everyone on drizzle-discuss is a > mystery > to me. > > docs/dirhtml/administration/authentication/index.html > - "there are no grant or privilege tables." Not quite true, since a plugin > could define some. What you are trying to say is that "by default there is > no single source where users are defined, such as a system user table, but > each authentication plugin will use different sources to verify the > usernames > and passwords. (The plugin auth_schema does however keep users in a table > inside Drizzle, much like the familiar MySQL way of authenticating > users works.)" > > - LDAP authentication can use the mysql protocol if you create the > drizzlePassword field and populate it with a hashed password. Edward (koko) > is writing the documentation on this. > > - Don't be shy here. In the third paragraph you should add a sentence saying > something like "If you are unsure what to do, you should enable the > auth_schema > plugin as this is the simplest yet secure method to enable user > authentication. > See the auth_schema documentation for details." > > - Instead of just linking to each plugin, it would give a nice overview to > have > a short description of each. Example: > PAM Authentication allows you to connect to Drizzle using your Linux > username > and password. > > docs/dirhtml/administration/authorization/index.html > - "there are no grant or privilege tables." Same comment here. > > - Add sentence "Currently there is no plugin that would implement an > authorization > scheme using the standard GRANT and REVOKE keywords familiar from all other > SQL databases. However, the internal authorization plugin API would fully > support developing such a plugin, if someone wants to develop it." (Eh... > I may have spoken to soon here :-( > > - This is a comment on the auth_regex plugin implementation: The pair of > commands ACCEPT/DENY is confusing. It should have been either ACCEPT/REJECT > (see iptables) or ALLOW/DENY (see apache httpd). Possibly I will > file a bug, possibly not... > > - Eh, is it true our Authorization API is just concerned about "access" and > doesn't separate, for instance, reads and writes? > > docs/dirhtml/administration/logging/index.html > - Same here, need to check if syslog is in fact enabled by default now. > > - The text already links to various plugins, but perhaps add a list at the > end with all logging plugins in alphabetical order (or preferred order?). > > docs/dirhtml/plugins/query_log/index.html > - "Log queries that cause more than N errors." Errors or warnings? > > docs/dirhtml/administration/plugins/index.html > - How did you come up with the list of default plugins? At least JS is > missing. > > docs/dirhtml/replication/drizzle/index.html > - Eh, you've moved this file, but apparently didn't actually edit it? > This looks like internals documentation to me, it doesn't explain how one > would actually setup replication. > - There should be a separate chapter on internals. Also a lot of Wiki stuff > needs to be moved there. > > > henrik > -- > [email protected] > +358-40-8211286 skype: henrik.ingo irc: hingo > www.openlife.cc > > My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559 > -- [email protected] +358-40-8211286 skype: henrik.ingo irc: hingo www.openlife.cc My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559
=== modified file 'docs/configuration/options.rst' --- docs/configuration/options.rst 2011-10-23 05:45:09 +0000 +++ docs/configuration/options.rst 2011-10-30 15:28:31 +0000 @@ -13,7 +13,7 @@ #. :ref:`command_line_options` Values from :ref:`command_line_options` have the highest precedence; -they override values from :ref:`config_files` which override the defaul +they override values from :ref:`config_files` which override the default values. The default values alone are sufficient to start :program:`drizzled`, but since they provide only the most basic configuration, you will certainly want to specify other options. @@ -146,7 +146,7 @@ A good strategy for configuring Drizzle with multiple config files is to put :ref:`drizzled_options` in :file:`/etc/drizzle/drizzled.cnf` (:file:`/etc/drizzle` is the default :option:`--config-dir` value) -and plugin options in spearate config files in +and plugin options in separate config files in :file:`/etc/drizzle/conf.d/`. For example: .. code-block:: bash @@ -182,7 +182,7 @@ Boolean options do not and cannot take values. Most boolean options are disabled by default, so specifying them enables them. -For example, ``--transaction-log.enable`` enable the transaction log because +For example, ``--transaction-log.enable`` enables the transaction log because it is disabled by default. However, some options are *enabled* by default, so specifying them disables them. For example, ``--innodb.disable-checksums`` disables InnoDB checkum validation because it is enabled by default. === modified file 'docs/configuration/variables.rst' --- docs/configuration/variables.rst 2011-10-23 05:45:09 +0000 +++ docs/configuration/variables.rst 2011-10-30 15:31:02 +0000 @@ -10,7 +10,7 @@ whereas :ref:`configuration_options` configure Drizzle at startup and cannot be changed without restarting Drizzle. -To see which variables are avaiable, execute the following: +To see which variables are available, execute the following: .. code-block:: mysql @@ -50,8 +50,8 @@ #. Dynamic #. Scope -Variable are created either by the Drizzle kernel or by a plugin. -Varaibles created by a plugin are prefixed with the plugin's name. +Variables are created either by the Drizzle kernel or by a plugin. +Variables created by a plugin are prefixed with the plugin's name. For example, ``drizzle_protocol_port`` is created by the :doc:`/plugins/drizzle_protocol/index` plugin. Else, the variable is created by the Drizzle kernel. === modified file 'plugin/query_log/docs/index.rst' --- plugin/query_log/docs/index.rst 2011-10-23 05:45:09 +0000 +++ plugin/query_log/docs/index.rst 2011-10-30 16:31:49 +0000 @@ -86,7 +86,7 @@ :Default: ``0`` :Variable: ``query_log_threshold_session_time`` - Log queries form sessions active longer than ``MICROSECONDS``. + Log queries from sessions active longer than ``MICROSECONDS``. The query log file writes ``session_time`` as seconds with six decimal place of precision, but this option must specify a number of microseconds. See :ref:`converting-seconds-to-microseconds`. @@ -282,7 +282,7 @@ Events are separated by a single ``#`` character. This record separator can be used by programs like :program:`awk` and :program:`perl` to easily separate events in a log. -The first line of each event has UTC/GMT timestamps with microsecond precision; the timezone cannot be changed. The second line has attributes with integer values. The third line has attributes with high-precision time values, always with six decimals places of precision. The fourth line has attributes with boolean values, either ``true`` or ``false``. The fifth line has attributes with string values, always double-quoted. Remaining lines are the query which can contain multiple lines, blank lines, et. The record separator marks the event of the event. +The first line of each event has UTC/GMT timestamps with microsecond precision; the timezone cannot be changed. The second line has attributes with integer values. The third line has attributes with high-precision time values, always with six decimals places of precision. The fourth line has attributes with boolean values, either ``true`` or ``false``. The fifth line has attributes with string values, always double-quoted. Remaining lines are the query which can contain multiple lines, blank lines, etc. The record separator marks the event of the event. As the example above demonstrates, the meta-format for each event in the query log is:: @@ -314,7 +314,7 @@ The authoritative source for issues, bugs and updated information is always `Drizzle on Launchpad <https://launchpad.net/drizzle>`_, but this is a list of notable bugs and limitations at the time of writing of which you should be aware before using this plugin. -* Error handling and reporting is not the best. This mostly affects changing ``query_log_file``. If you try to use a file that cannot be opened, the query log plugin prints an error to ``STDERR`` and disabled ``query_log_file_enabled``. +* Error handling and reporting is not the best. This mostly affects changing ``query_log_file``. If you try to use a file that cannot be opened, the query log plugin prints an error to ``STDERR`` and sets ``query_log_file_enabled`` to disabled. * ``lock_time`` is broken, wrong. See https://bugs.launchpad.net/drizzle/+bug/779708. * If the query log file is removed or changed while open (i.e. while ``query_log_file_enabled`` is true), it will not be recreated and query logging will stop. You need to disable and re-enable the log file to restore logging. @@ -328,7 +328,7 @@ 0.5 second * 1000000 = 500000 microseconds -To convert back, multiple the number of microseconds by ``0.000001`` (that's zero point five zeros and a one). +To convert back, multiply the number of microseconds by ``0.000001`` (that's zero point five zeros and a one). .. _query_log_authors:
_______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
