[U-Boot] [PATCH 4/6] Unify active vs. redundant environment variable naming

Wolfgang Denk wd at denx.de
Sun Aug 31 20:57:30 CEST 2008


Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0808311810110.3747 at axis700.grange> you wrote:
> 
> It's not purely stylistic. For example, previously the code had fd and 
> fdr, curdev and otherdev. It used one erase struct for both main and 
> redundant copies, thus they had to initialise it multiple times to one or 
> another version. I separated it into two erase_current and erase_target 
> thus removing the need for multiple initialisation. I think, having 

Ah! So there was a functional change (but - what was this needed
for?), which esceaped me because it was buried across all those
variable renamings.

> dev_target, erase_target, fd_target vs. dev_current, erase_current and 
> fd_current is also a readability improvement.
> 
> > I reject this patch.
> 
> Please, reconsider.

I still reject it, at least as is. Whether  you  call  the  variables
curdev  and  otherdev  versus  dev_current  and  dev_target  makes no
significant change to me, except that the new names are longer,  more
difficult  to  type  and  to  read. And any functional changes become
completely invisible among all the renaming. This makes such  patches
unacceptable to me.

It is important that you can actually SEE what a patch  is  changing.
With  your  patches, this is not the case. You change everything, and
the significant modifications become invisible.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If it happens once, it's a bug.
If it happens twice, it's a feature.
If it happens more than twice, it's a design philosophy.


More information about the U-Boot mailing list