Re: [PATCH 1/7] path.c: implement xdg_config_home()
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 13, 2015, 15:50 UTC
- Message-ID
- <e1bc6f19af608db796a2212dbf00ba45@www.dscho.org>
- In-Reply-To
- <1428824772-8736-1-git-send-email-pyokagan@gmail.com>
Hi Paul,
maybe it would be a good idea to add a `0/7` mail that describes the overall goal of this patch series, much like a Pull Request? I found it very useful -- even for myself -- to set a description via `git branch --edit-description` and to let `git format-patch` use that via the `--cover-letter` option.
below just two minor nits because the rest of the patches looks fine to me:
On 2015-04-12 09:46, Paul Tan wrote:
Show 15 quoted lines
> diff --git a/cache.h b/cache.h > index 3d3244b..7f9bab0 100644 > --- a/cache.h > +++ b/cache.h > @@ -836,6 +836,13 @@ char *strip_path_suffix(const char *path, const > char *suffix); > int daemon_avoid_alias(const char *path); > extern int is_ntfs_dotgit(const char *name); > > +/** > + * Returns the newly allocated string "$XDG_CONFIG_HOME/git/%s". If > + * $XDG_CONFIG_HOME is unset or empty, returns the newly allocated string > + * "$HOME/.config/git/%s". Returns NULL if an error occurred. > + */ > +extern char *xdg_config_home(const char *fn);
Should this not be inserted close to home_config_paths()? Also, the name "fn" sounds more like "function" than like "filename" to me, especially keeping the name `config_fn_t` in mind. Maybe call the parameter "filename" to avoid confusion?
Thanks, Dscho