[m-rev.] for post-commit review: touched_files

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Dec 8 12:54:41 AEDT 2023


On 2023-12-08 12:42 +11:00 AEDT, "Peter Wang" <novalazy at gmail.com> wrote:
>> +    TouchedFiles = touched_files(TouchedTargetFiles, TouchedTimestampFiles),
>>      list.map_foldl2(
>>          get_target_timestamp(ProgressStream, Globals, do_not_search),
>>          TouchedTargetFiles, TargetTimestamps, !Info, !IO),
> 
> The name "TouchedTimestampFiles" (here and elsewhere),
> when the list may include non-timestamp files, could be confusing.

Do you have a more descriptive name that is not too long? I tried to think of one,
but failed. Basically, *most* of those files *are* timestamp files, but *all* of them,
even the ones that are e.g. .c or .o files, are files that are passed along so that
code can call "stat" on them and see when they were last modified. In *that* sense,
though not in the usual one, they are timestamp files, and the comment on
the second field of the new type should explain that fact.

I was actually more concerned about the other part of the name, "Touched".
The code that computes the touched_files structure does not actually touch
any of those files; it just computes the set of files that building the chosen
target *would* touch. Again, I tried and failed to find a name that expresses
that fact while not being too long, so I kept the old naming scheme while
explaining the semantics in the new type's comment.

> Other than that, the diff is fine.

Thanks.

Zoltan.


More information about the reviews mailing list