[PATCH] util: rename _windows_reserved_filenames and _windows_reserved_chars

Jason Harris jason at jasonfharris.com
Sun May 8 08:06:15 CDT 2011


On May 8, 2011, at 2:37 PM, Adrian Buehlmann wrote:

> On 2011-05-08 14:16, Laurens Holst wrote:
>> Op 8-5-2011 13:07, Adrian Buehlmann schreef:
>>> On 2011-05-08 11:55, Steven Brown wrote:
>>>> Suggestion: The variables are only used within the checkwinfilename
>>>> function, so declare them within the function. The "windows" part of
>>>> the variable names is now implied. The variables could either be
>>>> inlined, or renamed: reservedfiles, reservedchars.
>>> Isn't defining the constants outside the function more efficient (in terms
>>> of program execution time)? (assuming the function is called more than once
>>> throughout a program run).
>>> 
>>> FWIW, I think reservedfiles is a bit misleading.
>>> 
>>> However, I'm not opposed to using something like _winreservednames instead
>>> of _winresnames, if people feel more comfortable with that.
>> 
>> Yes _winreservednames was exactly what I wanted to suggest. You could 
>> also leave out the win part as Steven suggested: _reservednames.
> 
> The "win..." is mostly there, because the name is defined at the util
> module *global* scope.
> 
> This piece of code deals with Windows only. _reservednames on global
> scope feels a bit too bold, per my taste.
> 
>> The problem I think with having to interpret abbreviations in context is 
>> that you actually have to spend time reading the context, find the 
>> warning a couple of lines down and think ‘oh, that’. In general I try to 
>> avoid abbreviations (then again, I mostly write Java(Script)).
> 
> Yeah. This is not like we are suddenly start adopting Java folklore
> around here.
> 
> (Amazing how much dust this patch raises.)

Arian why are you so hostile here? Why hassle Laurens because he writes
in Java and Javascript? Laurens' reasoning is totally valid here and his
reasoning is language independent...

People are likely commenting on this patch because the change

   _windows_reserved_filenames  --->  _winresnames

is a pretty stark regression in code readability, and it illustrates the
problems with the current code styling conventions.

Cheers,
  Jas


More information about the Mercurial-devel mailing list