Code Documentation Conventions

Code Documentation Conventions

[Josh Durgin]

I've been documenting the librados C api, and would like to get some
feedback on the format being used. For each function, the format is
generally:

/**
  * Short description
  *
  * Detailed description when necessary
  *
  * preconditons, postconditions, warnings, bugs or other notes
  *
  * parameter reference
  * return value (if non-void)
  */

Below is some sample documentation. The markup is doxygen, although
I've had to avoid some directives since breathe (sphinx plugin for
reading doxygen output) removes contents it doesn't understand.

For higher-level comments, I've been using named sections, and grouping
related functions under them (this would be using @defgroup, but
breathe doesn't seem to support that). @{ and @} delimit a group.

/**
  * @name Configuration
  * These functions read and update Ceph configuration for a cluster
  * handle. Any configuration changes must be done before connecting to
  * the cluster.
  *
  * Options that librados users might want to set include:
  * - mon_host
  * - auth_supported
  * - key, keyfile, or keyring when using cephx
  * - log_file, log_to_stderr, err_to_stderr, and log_to_syslog
  * - debug_rados, debug_objecter, debug_monc, debug_auth, or debug_ms
  *
  * All possible options can be found in src/common/config_opts.h in 
ceph.git
  *
  * @{
  */

/**
  * Configure the cluster handle using a Ceph config file
  *
  * If path is NULL, the default locations are searched, and the first
  * found is used. The locations are:
  * - $CEPH_CONF (environment variable)
  * - /etc/ceph/ceph.conf
  * - ~/.ceph/config
  * - ceph.conf (in the current working directory)
  *
  * @pre rados_connect() has not been called on the cluster handle
  *
  * @param cluster cluster handle to configure
  * @param path path to a Ceph configuration file
  * @returns 0 on success, negative error code on failure
  */
int rados_conf_read_file(rados_t cluster, const char *path);

/**
  * Configure the cluster handle with command line arguments
  *
  * argv can contain any common Ceph command line option, including any
  * configuration parameter prefixed by '--' and replacing spaces with
  * dashes or underscores. For example, the following options are 
equivalent:
  * - --mon-host 10.0.0.1:6789
  * - --mon_host 10.0.0.1:6789
  * - -m 10.0.0.1:6789
  *
  * @pre rados_connect() has not been called on the cluster handle
  *
  * @param cluster cluster handle to configure
  * @param argc number of arguments in argv
  * @param argv arguments to parse
  * @return 0 on success, negative error code on failure
  */
int rados_conf_parse_argv(rados_t cluster, int argc, const char **argv);

/**
  * Configure the cluster handle based on an environment variable
  *
  * The contents of the environment variable are parsed as if they were
  * Ceph command line options. If var is NULL, the CEPH_ARGS
  * environment variable is used.
  *
  * @pre rados_connect() has not been called on the cluster handle
  *
  * @bug this is not threadsafe - it uses a static buffer
  *
  * @param cluster cluster handle to configure
  * @param var name of the environment variable to read
  * @return 0 on success, negative error code on failure
  */
int rados_conf_parse_env(rados_t cluster, const char *var);

/**
  * Set a configuration option
  *
  * @pre rados_connect() has not been called on the cluster handle
  *
  * @param cluster cluster handle to configure
  * @param option option to set
  * @param value value of the option
  * @return 0 on success, negative error code on failure. -ENOENT is
  * returned when the option is not a Ceph configuration option.
  */
int rados_conf_set(rados_t cluster, const char *option, const char *value);

/**
  * Get the value of a configuration option
  *
  * @param cluster configuration to read
  * @param option which option to read
  * @param buf where to write the configuration value
  * @param len the size of buf in bytes
  * @return 0 on success, negative error code on failure.
  * -ENAMETOOLONG is returned if the buffer is too short to contain the
  * requested value.
  */
int rados_conf_get(rados_t cluster, const char *option, char *buf, 
size_t len);

/** @} config */

Re: Code Documentation Conventions

[Josh Durgin]

On 12/16/2011 12:52 PM, Josh Durgin wrote:
> I've been documenting the librados C api, and would like to get some
> feedback on the format being used. For each function, the format is
> generally:
>
> /**
> * Short description
> *
> * Detailed description when necessary
> *
> * preconditons, postconditions, warnings, bugs or other notes
> *
> * parameter reference
> * return value (if non-void)
> */
>
> Below is some sample documentation. The markup is doxygen, although
> I've had to avoid some directives since breathe (sphinx plugin for
> reading doxygen output) removes contents it doesn't understand.

You can see the sphinx ouptut at:
http://www.joshd.dreamhosters.com/api/librados/#project0librados_8h_1a280e071e73202dca849472814a585717

The function reference all comes from doxygen's processing of 
src/include/rados/librados.h, and the code examples (which I haven't 
changed yet) are from doc/api/librados.rst. In general, I think we 
should document functions in header files and only put code examples or 
larger usage guides outside the code.

I've been focused on making the content correct, so I haven't customized 
the sphinx or doxygen configuration at all. There are lots of things we 
could improve about the output. Also I just noticed breathe is ignoring 
doxygen lists.

Re: Code Documentation Conventions

[Tommi Virtanen]

On Fri, Dec 16, 2011 at 12:52, Josh Durgin <josh.durgin@dreamhost.com> wrote:
> I've been documenting the librados C api, and would like to get some
> feedback on the format being used. For each function, the format is
> generally:
>
> /**
>  * Short description
>  *

Personally I've never been crazy about the "*" prefix on every line,
it looks just like extra work without value to me. Doxygen itself is
perfectly happy with just

/**
  Short description

  blah
  blah
 */

But whatever makes sense for you guys.
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Code Documentation Conventions

[Gregory Farnum]

On Mon, Dec 19, 2011 at 10:08 AM, Tommi Virtanen
<tommi.virtanen@dreamhost.com> wrote:
> On Fri, Dec 16, 2011 at 12:52, Josh Durgin <josh.durgin@dreamhost.com> wrote:
>> I've been documenting the librados C api, and would like to get some
>> feedback on the format being used. For each function, the format is
>> generally:
>>
>> /**
>>  * Short description
>>  *
>
> Personally I've never been crazy about the "*" prefix on every line,
> it looks just like extra work without value to me. Doxygen itself is
> perfectly happy with just
>
> /**
>  Short description
>
>  blah
>  blah
>  */
>
> But whatever makes sense for you guys.
I like it as a visibility and "comment type" thing. Plus any good
editor will auto-type those for you... ;)
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Code Documentation Conventions

[Tommi Virtanen]

On Mon, Dec 19, 2011 at 10:13, Gregory Farnum
<gregory.farnum@dreamhost.com> wrote:
>> Personally I've never been crazy about the "*" prefix on every line,
> I like it as a visibility and "comment type" thing. Plus any good
> editor will auto-type those for you... ;)

And on the flip side, any good editor will auto-highlight the /** ...
*/ for you.. ;)

Re: Code Documentation Conventions

[Sage Weil]

On Mon, 19 Dec 2011, Tommi Virtanen wrote:
> On Mon, Dec 19, 2011 at 10:13, Gregory Farnum
> <gregory.farnum@dreamhost.com> wrote:
> >> Personally I've never been crazy about the "*" prefix on every line,
> > I like it as a visibility and "comment type" thing. Plus any good
> > editor will auto-type those for you... ;)
> 
> And on the flip side, any good editor will auto-highlight the /** ...
> */ for you.. ;)

All things being equal, I'd go for * just because that's what everybody 
else does.

Josh, this looks good to me!

sage