Activity

Here are suggestions on how to improve code comments of the Thread manager component so that Doxygen parses them correctly.

GENERAL NOTE: I do respect the authors of all these headers and I do understand how proud they are to see their names in the comments! But, since it's the open source, I'd like to ask you to remove the authors' names from the files content. Thanks in advance and sorry about that!

Svetlana Konovalova
added a comment - 11/Apr/07 07:28 2include_headers.patch corrects grammar and fixes formatting in include/open/compmgr.h and include/component_manager.h
Here are suggestions on further improvement:
include/open/compmgr.h
Define parameters as [in] / [out]
Add member data documentation descriptions
not sure it should be this way:
"instance - a handle of an instance to free"
Maybe <code>FREE</code>, or what?
component_manager.h
add brief/detailed descriptions
define parameters as [in] / [out]
Would be great if someone could take care of the aforementioned files.
Thanks in advance,
Sveta

The sentence becomes incorrect. The system won't initialize a component manager as well if another one exists. I asked Nadya to provide a reference to the non-Intel documentation which prefixes "managers" with "-".

Alexei Fedotov
added a comment - 04/Jun/07 11:46 Thank you for your corrections. Adding my comments concerning the fixes.
1. What is the reason behind the following change?
/**
@ingroup Handles
* The handle of the open component.
+ * The open component handle.
*/
Probably "of" can be replaced with "for" in my description, and there should be undefined articles. I intentionally put the long form here to outline that this is a handle.
2. What is rationale for the following change? License text is not a javadoc.
-/*
+/**
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
3.
/**
* If no component manager exist, initializes a component manager.
* Otherwise, increases a component manager reference count.
+ * Initializes a component manager if it does not exist.
+ * Otherwise, increases a component-manager reference count.
The sentence becomes incorrect. The system won't initialize a component manager as well if another one exists. I asked Nadya to provide a reference to the non-Intel documentation which prefixes "managers" with "-".
4.
*
* @param init_func - initializer function which provides a default
+ * @param init_func - the initializer function providing a default
and private interfaces for the component
An initializer function? Why we use "the" for non-defined term?
5. I'm ok with all following changes
/**
* Decrement a reference counter and destroy a component manager if it
+ * Decrements a reference counter and destroys a component manager if it
becomes zero. The caller should ensure no cached handles will be used.
*
6. It seems that the patch doesn't have correct line endings.
-/**
+
+/**
7. What is the reason behind this change?
/**
@name Handles
* Handles are opaque for an interface user. The user
* operates with handles by means of interface functions.
+ * Handles are opaque for an interface user. The user operates with handles
+ * by means of interface functions.
*/
8. A format is changed to the incorrect one.
/**
* Returns a component version which is numbers separated with dots.
* Implementors must check a major version number for compatibility.
*/
+/**
+ * Returns a component version where numbers are separated with dots.
+ * Implementors must check the major version number for compatibility.
+ */

1 - ok, the intention was presumably to make the phrase shorter; suggest that we stick with the original, "of" > "for" change is not recommended

2 - we've talked about it : if you have a non-Doxygen comment, Doxygen does not parse it so when opening a Source view of the parsed file and expect to get a clean version, you see the disclaimer; on the other hand, if you format the disclaimer so that Doxygen can digest it, the comment disappears from the source view but is not attached to any code entity; using normal Doxygen comment notation for disclaimers has been a common practice for quite a while on Harmony

3 - about incorrect wording - i disagree with Alexei; the phrase "Initializes a component manager if it does not exist. " can only be interpreted as it reads - if there has been no component manager, it is created; the new version of the phrase is shorter and does not repeat "component manager" two times

3 - about compound nouns - indeed, the hyphen is widely used and helps avoid ambiguity; a quote from a well-known and respected book on technical writing Microsoft Manual of Style for Technical Publications: " Hyphenate two or more words that precede and modify a noun as a unit if confusion might otherwise result.
Use memory-resident program, not TSR, in content for home users and information workers.
Some programs have dialog box-type options for frequently used operations.
Other examples include items such as "arguments, command line" and "command-line arguments."
i don't think this is a discussion of English grammar, but if you have more questions, i'd be happy to help and consult. Really

4. Because we define a specific parameter, it passes the name of a specific function, and everything specific is definite - hence, use a definite article If you said "an initializer function", you'd mean any, does not matter which -but it does, it's code right there.

6. yes, we should probably run a check for dos2unix and tabs-to-spaces fixes before supplying any patch

7. i haven't found differences in wording, so i presume, the change is done to limit comment-line length to 80 characters. Sveta, correct me if you had some other fix in mind.

8. i guess it's for unification purposes; we don't usually indent the first comment line; this is not a strict rule, but whichever style you choose, please follow through the whole file. comments that are formatted inconsistently should be brought to a unified manner.

Alexei,
please let me know if you have more questions. What should we do with the patch - once you're satisfied with its quality?
thanks,
looking forward to your reply, Nadya

Nadya Morozova
added a comment - 25/Jun/07 12:46 a couple of comments on questions from Alexei:
1 - ok, the intention was presumably to make the phrase shorter; suggest that we stick with the original, "of" > "for" change is not recommended
2 - we've talked about it : if you have a non-Doxygen comment, Doxygen does not parse it so when opening a Source view of the parsed file and expect to get a clean version, you see the disclaimer; on the other hand, if you format the disclaimer so that Doxygen can digest it, the comment disappears from the source view but is not attached to any code entity; using normal Doxygen comment notation for disclaimers has been a common practice for quite a while on Harmony
3 - about incorrect wording - i disagree with Alexei; the phrase "Initializes a component manager if it does not exist. " can only be interpreted as it reads - if there has been no component manager, it is created; the new version of the phrase is shorter and does not repeat "component manager" two times
3 - about compound nouns - indeed, the hyphen is widely used and helps avoid ambiguity; a quote from a well-known and respected book on technical writing Microsoft Manual of Style for Technical Publications: " Hyphenate two or more words that precede and modify a noun as a unit if confusion might otherwise result.
Use memory-resident program, not TSR, in content for home users and information workers.
Some programs have dialog box-type options for frequently used operations.
Other examples include items such as "arguments, command line" and "command-line arguments."
i don't think this is a discussion of English grammar, but if you have more questions, i'd be happy to help and consult. Really
4. Because we define a specific parameter, it passes the name of a specific function, and everything specific is definite - hence, use a definite article If you said "an initializer function", you'd mean any, does not matter which -but it does, it's code right there.
6. yes, we should probably run a check for dos2unix and tabs-to-spaces fixes before supplying any patch
7. i haven't found differences in wording, so i presume, the change is done to limit comment-line length to 80 characters. Sveta, correct me if you had some other fix in mind.
8. i guess it's for unification purposes; we don't usually indent the first comment line; this is not a strict rule, but whichever style you choose, please follow through the whole file. comments that are formatted inconsistently should be brought to a unified manner.
Alexei,
please let me know if you have more questions. What should we do with the patch - once you're satisfied with its quality?
thanks,
looking forward to your reply, Nadya