Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Feb 7, 2018, 22:06 UTC
- Message-ID
- <20180207140638.a4ff04513e10e1fc3f2d7c9c@google.com>
- In-Reply-To
- <20180205235735.216710-5-sbeller@google.com>
On Mon, 5 Feb 2018 15:54:46 -0800 Stefan Beller <sbeller@google.com> wrote:
Show 5 quoted lines
> + /* > + * Path to the alternate object database, relative to the > + * current working directory. > + */ > char path[FLEX_ARRAY];
I would prefer this to be commented:
Path to the alternative object store. If this is a relative path, it is relative to the current working directory.
to show that it is not necessarily relative, but the current version is fine too.
Show 7 quoted lines
> + /* > + * Paths in alt are relative to the cwd. We ignore environment > + * settings like this for all repositories except for > + * the_repository, so we don't have to worry about transforming > + * the path to be relative to another repository. > + */ > + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);
I find the comment confusing - it makes more sense for the reason for us not worrying about transforming the path is that the paths as stored in struct alternate_object_database are relative to the CWD, not that we ignore environment variables for certain repositories.
I think it's best to remove this comment, and instead add a comment to read_info_alternates() before its call to link_alt_odb_entries(), explaining that paths in the alternates file are relative to "info/alternates", not to the CWD (since that is the exceptional case).
All the patches prior to this look good. Thanks especially for the consistent naming convention of the patch titles.