[m-rev.] for review: standardize "reading file" messages

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Jan 21 17:59:28 AEDT 2022


2022-01-21 14:20 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>> +    % Why are we attempting to read and parse a file?
>> +:- type read_msg
> 
> Maybe name the type read_reason_msg or read_purpose_msg.

I went with read_reason_msg.

>> +    ;       rm_old(module_name)
>> +            % Because the named file belongs to this module, and we need
>> +            % the old contents of the file (i.e. the current contents
>> +            % that is about to be overwritten, if necessary).

That one now reads

 ;       rrm_old(module_name)
            % Because the named file belongs to this module, and we need
            % the old contents of the file, so that we can write out
            % what we *now* think its contents should be, but *only*
            % if the "new" contents differ from the old. The difference
            % between whether (a) we keep the old contents as is, or
            % (b) we overwrite them with identical contents, is the
            % timestamp on the file. Keeping the old timestamp will
            % prevent unnecessary rebuilds of other files that depend
            % on that one.

>> +    % - None of our callers are ever interested in timestamps,
>> +    %   from the module name, so these predicates do not search for the
>> +    %   right filename either, but t
> 
> "but t"?

Thanks for catching that, but actually, nothing on the last two lines makes sense :-(,
due to it being left over from a copy-and-pase. That comment now reads

% - None of our callers are ever interested in timestamps,
%   so these predicates never return them.

Thanks for the review.

Zoltan.


More information about the reviews mailing list