Thanks, for the patch, Chuck. You have done thorough work here, and
I do not envy you the job. It was probably not the most thrilling
experience ...
STATUS
======
Approved.
COMMENTARY
==========
Since the bug is about removing Doxygen errors and warnings, and I
have checked that you have done that, I approve this patch. However,
I feel that some of the added comments are just telling the obvious,
and I ask myself what is the point here. Some examples:
> + /// The equal comparison operator.
> bool operator==(const Db_ref &db) const
> { return stringcmp(m_name, &db.name()) == 0; }
This comment does not tell me anything that I could not infer by
looking at the code.
> + /**
> + Constructor for LEX_STRING class.
> +
> + @param[in] s LEX_STRING string.
> + */
> LEX_STRING(const ::LEX_STRING &s)
> {
> str= s.str;
> length= s.length;
> }
Nothing new here either ...
> + /// Return the name.
> const String& name() const
> { return *m_name; }
I could have guessed this, too.
Do we have to add such dummy statements to avoid warnings? I think if
nothing more is needed to be said, it is better to just have an empty
comment. The way it is now it violates normal principles of comments
in code. (The comments should have a purpose, and not document the
obvious).
I also wonder if it is right to have a specific Doxyfile for backup.
Would it not be better to have the same definitions for all the MySQL
code?
SUGGESTIONS
===========
> +/// Type definition of version_t.
> typedef uint version_t;
Here I would have expected some explanation of what kind of number
version_t represent. E.g., what context it is to be used in. The are
more cases like this. I can't remember seeing any typedef comment
that did not just state the obvious.
--
Øystein
Chuck Bell wrote:
> #At file:///C:/source/bzr/mysql-6.0-bug-40653/ based on
revid:ingo.struewing@stripped
>
> 2735 Chuck Bell 2008-12-15
> BUG#40653 Numerous doxygen errors and warnings in backup code
>
> This patch corrects a large number of doxygen errors and warnings.
> The type of things changed include (but no logic changes)
> * documentation of attributes, typedefs, constants
> * corrected documentation of methods
> * code alignment issues (where appropriate)
> * change of /** to ///< for uniformity
...
