git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/6] Document a bunch of functions defined in sha1_file.c

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Feb 24, 2014, 20:08 UTC
Message-ID
<20140224200855.GI7855@google.com>
In-Reply-To
<530BA530.3070603@alum.mit.edu>
Hi,
Michael Haggerty wrote:
Show 18 quoted lines
> No, this hasn't changed.  I've been documenting public functions in the
> header files above the declaration, and private ones where they are
> defined.  So I moved the documentation for this function to cache.h:
>
> +/*
> + * Return the name of the file in the local object database that would
> + * be used to store a loose object with the specified sha1.  The
> + * return value is a pointer to a statically allocated buffer that is
> + * overwritten each time the function is called.
> + */
>  extern const char *sha1_file_name(const unsigned char *sha1);
>
> I also rewrite the comment, as you can see.  The "NOTE!" seemed a bit
> overboard to me, given that there are a lot of functions in our codebase
> that behave similarly.  So I toned the warning down, and tightened up
> the comment overall.
>
> Let me know if you think I've made it less helpful.

In the present state of the codebase, where many important functions have no documentation or out-of-date documentation, the first place I look to understand a function is its point of definition. So I do think that moving docs to the header file makes it less helpful. I'd prefer a mass move in the opposite direction (from header files to the point of definition).

On the other hand I don't feel strongly about it.

Hope that helps, Jonathan

Previous: Michael HaggertyNext: Michael Haggerty
Message 17 of 23 in “Add a bunch of docstrings and make a few minor cleanups”
  1. 0/6 Add a bunch of docstrings and make a few minor cleanupsMichael Haggerty, Feb 21, 2014
  2. 1/6 Add docstrings for lookup_replace_object() and do_lookup_replace_object()Michael Haggerty, Feb 21, 2014
  3. Junio C HamanoFeb 21, 2014
  4. Michael HaggertyFeb 24, 2014
  5. Christian CouderFeb 24, 2014
  6. Michael HaggertyFeb 24, 2014
  7. Junio C HamanoFeb 24, 2014
  8. 2/6 replace_object: use struct members instead of an arrayMichael Haggerty, Feb 21, 2014
  9. Junio C HamanoFeb 21, 2014
  10. 3/6 find_pack_entry(): document last_found_packMichael Haggerty, Feb 21, 2014
  11. Nicolas PitreFeb 21, 2014
  12. 4/6 sha1_file_name(): declare to return a const stringMichael Haggerty, Feb 21, 2014
  13. 5/6 Document a bunch of functions defined in sha1_file.cMichael Haggerty, Feb 21, 2014
  14. Nicolas PitreFeb 21, 2014
  15. Jakub NarębskiFeb 24, 2014
  16. Michael HaggertyFeb 24, 2014
  17. Jonathan NiederFeb 24, 2014
  18. Michael HaggertyFeb 25, 2014
  19. 6/6 Document some functions defined in object.cMichael Haggerty, Feb 21, 2014
  20. Nicolas PitreFeb 21, 2014
  21. Michael HaggertyFeb 24, 2014
  22. Junio C HamanoFeb 24, 2014
  23. Junio C HamanoFeb 24, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.