Goals for this project:

  • Describe what the function does. Do not describe what the function is used for.
  • The following forms should be used:
    • method_table::method_name
    • function_name()
    • array_name[]
  • Add a description at the start of the file that gives the reader an overall idea of what the functions in this file are for, and a bit of background on the code itself.  This is expected to be at least a paragraph or two, but can be considerably longer if the file is important for the code Lustre operation, or implements a complex algorithm that may not be easily understood just from reading the functions
  • Update the Intel copyright line at the top of the file to 2014.  If there is an Oracle or Sun copyright line, or a notice about the Lustre trademark, it should be left alone.
  • Replace the Sun URL for the GPLv2 with http://www.gnu.org/licenses/gpl-2.0.html
  • Add a comment block for each function in the file that is more than two or three lines long. Include all important details for users of this function:
    • external locking requirements, or if function changes the locking state itself
    • requirements on buffer sizes, memory that is allocated or freed, etc.

See Coding Guidelines for details on the formatting of the function comments themselves.  The function comment should contain most or all of the comments for the function, so that the reader can understand what the function is doing, instead of having comments spread throughout the function.  Comments inline with the code should be short and be used to point out some tricky bit of implementation detail that needs to be explained.  For example:
 

/**
 * Implements method_table::method_name() method for subsystem.
 *
 * Provide a description of the function here, including any details
 * about the function that may be important.  This includes information
 * about locks held by the caller or other requirements that the function
 * has of the calling function and how \a var1 and \a var2 might be used.
 *
 * \param[in]     var1    description of this variable
 * \param[in]     var2    another description of a variable,
 *                        with more detail than fits on one line
 * \param[in,out] var3    this one is for both input and output 
 *
 * \retval        0       success
 * \retval        -EAGAIN need to call this function again
 * \retval        -ENOMEM error allocating some structure
 * \retval        negative error number for other errors
 */

 

  • As you work through the functions in a file, you may see other technical debts that should be fixed.  While it is OK to fix a bit of whitespace, do NOT fix everything in the documentation patch, since this will make it more difficult to inspect the patch and ensure that it does not introduce some problem itself. Make separate patches to fix problems found in the code, and/or keep a separate list of issues that need to be fixed later.
  • If there is some part of the code in the file that you are working on that you don't completely understand, look who else worked on the function recently using "git blame path/to/file.c" then chat or call someone in Skype to discuss the code and get a good understanding of it to write a good comment.  Failing that, write what you think the function is doing, and then add the original authors as inspectors for the patch.
  • Don't worry if you don't understand everything 100% - it is better to have something written down that can be improved, than having nothing at all.
  • Don't worry about proper English grammar.  That can be fixed up by others during patch inspection, once the technical content is written.
  • Once you have a patch, push it to Gerrit and select inspectors that also are familiar with the code in this file (also using "git blame path/to/file.c" to see other developers who worked on the file if needed).
     

 

  • No labels