Re: [PATCH v2/GSoC 2/4] path.c: implement xdg_runtime_dir()

On Fri, Mar 18, 2016 at 12:48:44AM +0800, Hui Yiqun wrote:

this function does the following:

1. if $XDG_RUNTIME_DIR is non-empty, `$XDG_RUNTIME_DIR/git` is used in next
step, otherwise `/tmp/git-$uid` is taken.
2. ensure that above directory does exist. what's more, it must has correct
permission and ownership.
3. a newly allocated string consisting of the path of above directory and
$filename is returned.

Under following situation, NULL will be returned:
+ the directory mentioned in step 1 exists but have wrong permission or
ownership.
+ the directory or its parent cannot be created.

Notice:

+ the caller is responsible for deallocating the returned string.

I see a lot of "what" in your commit message (and in the other ones in
this series), but not a lot of "why".

We can see the "what" from the diff already (though it is certainly OK
to point out tricky parts). But you probably want to explain the
motivation for things, alternatives considered, etc, like:

  - why is using $XDG_RUNTIME_DIR a good thing?

  - why did you choose to fall back to /tmp (as opposed to, say,
    returning NULL and letting the caller handle it)

  - what is the purpose of the ownership/permission rules? You link to
    the XDG spec in an in-code comment, but IMHO that kind of motivation
    probably makes more sense in the commit message.

-Peff