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

[Guennadi Liakhovetski]

On Sun, 31 Aug 2008, Wolfgang Denk wrote:

> 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.

Well, as I worked on this patch series, it was pretty difficult to me to 
recognise which of the two environment copies the current code was dealing 
with. So, to help myself and any future developers, that will work with 
this code, I decided to make this distinction clearer and more consistent. 
Sorry, but to me it wasn't obvious, that fd was referring to devcur, and 
fdr to devother - just from the naming. Whereas using fd_current, 
dev_current and fd_target, dev_target makes it clearer, IMHO. Yes, this is 
longer to type, and produces longer lines. But I am prepared to pay this 
price:-)

As for the "functional" change - introducing an additional erase variable 
to avoid having to reuse and reinitialise one several times - it is a 
related change, it also separates handling of the two environment copies. 
So, I think, they belong to one patch.

Would it suffice to change the patch description for this patch to be 
accepted, or do you still want this patch to be dropped / changes? We 
could use fdcur, fdtrg, devcur, devtrg, erasecur, erasetrg to save the 
typing, but, personally, I find dev_current easier to read.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de