Christian Couder <christian.couder@xxxxxxxxx> writes:
> When we know that mtime is fully supported by the environment, we
> might want the untracked cache to be always used by default without
> any mtime test or kernel version check being performed.
>
> Also when we know that mtime is not supported by the environment,
> for example because the repo is shared over a network file system,
> we might want the untracked-cache to be automatically remove from
> the index.
I'd say "fully supported" and "not supported" are bad phrasing.
Network file system may give you mtime, but for the purpose of uc,
it may not be usable, and that is when you do not want to rely on
it. So perhaps "When we know that mtime on directory the
environment gives us is usable for the purpose of untracked cache",
and "Also when we know that mtime is not usable" or something like
that?
I think "we might want" can become a stronger "we may want" in both
paragraphs. The second paragraph needs s/remove/removed/.
> The normal way to achieve the above in Git is to use a config
> variable. That's why this patch introduces "core.untrackedCache".
>
> This variable is a tristate, `keep`, `false` and `true`, which
> default to `keep`.
The above makes a reader wonder what are abnormal ways ;-).
Allow the user to express such preference by setting the
'core.untrackedCache' configuration variable, which can take
'keep', 'false', or 'true'.
would be clearer without being muddied by an unnecessary value
judgement.
> When read_index_from() is called, it now adds or removes the
> untracked cache in the index to respect the value of this
> variable. So it does nothing if the value is `keep` or if the
> variable is unset; it adds the untracked cache if the value is
> `true`; and it removes the cache if the value is `false`.
OK.
> `git update-index --[no-|force-]untracked-cache` still adds or
> removes the untracked cache from the index,
The above is meant to be a contraction of "... still adds UC to the
index or removes UC from the index", but reads as if the original
were "... still adds UC from the index or removes UC from the
index".
"... still adds UC to or removes from the index" perhaps?
> but this shows a
> warning if it goes against the value of core.untrackedCache,
> as the untracked cache will go back to its previous state the
> next time read_index_from() is called.
Not incorrect per-se, but
... shows a warning ... because the next time the index is read
UC will be added or removed if the configuration is set to do so.
may make it more clear that `keep` in core.untrackedCache does not
have to issue a warning (I saw your code correctly handles the
warning in this patch).
> Also `--untracked-cache` used to check that the underlying
> operating system and file system change `st_mtime` field of a
> directory if files are added or deleted in that directory. But
> those tests take a long time and there is now
> `--test-untracked-cache` to perform them.
> That's why, to be more consistent with other git commands, this
> patch prevents `--untracked-cache` to perform tests, so that
> after this patch there is no difference any more between
> `--untracked-cache` and `--force-untracked-cache`.
But because those tests take a long time, `--untracked-cache` no
longer performs them. Instead, there is now `--test-untracked-cache`
to perform the tests. This change makes `--untracked-cache` the
same as `--force-untracked-cache`.
> +Untracked cache
> +---------------
> +
> +This cache could speed up commands that involve determining untracked
> +files such as `git status`.
You are being a bit too modest by saying "could" here, I think. If
you do not want to be overly assertive, perhaps say "is meant to"?
> +This feature works by recording the mtime of the working tree
> +directories and then omitting reading directories and stat calls
> +against files in those directories whose mtime hasn't changed. For
> +this to work the underlying operating system and file system must
> +change the `st_mtime` field of directories if files in the directory
> +are added, modified or deleted.
A reader who read only this without reading the code would wonder if
the code works correctly after a untracked path is added to a
diretory A/B (the timestamp of B would be updated, while A would
stay the same) and we ask what are the untracked things inside the
whole tree. Of course we shouldn't "omit" scanning A down to B, but
it is a bit unclear that we didn't have such a design bug from this
description.
Unfortunately I don't have a good rephrasing suggestion.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html