From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/6] Unify active vs. redundant environment variable naming
Date: Sun, 31 Aug 2008 20:57:30 +0200 [thread overview]
Message-ID: <20080831185730.5798F242FF@gemini.denx.de> (raw)
In-Reply-To: <Pine.LNX.4.64.0808311810110.3747@axis700.grange>
Dear Guennadi Liakhovetski,
In message <Pine.LNX.4.64.0808311810110.3747@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.
next prev parent reply other threads:[~2008-08-31 18:57 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-27 15:52 [U-Boot] [PATCH 0/6] Support NAND in fw_printenv/fw_setenv Guennadi Liakhovetski
2008-08-27 15:52 ` [U-Boot] [PATCH 1/6] Convert fw_env.c to use a single environment image union Guennadi Liakhovetski
2008-08-31 14:36 ` Wolfgang Denk
2008-08-31 15:57 ` Guennadi Liakhovetski
2008-08-31 18:57 ` Wolfgang Denk
2008-08-27 15:52 ` [U-Boot] [PATCH 2/6] Separate flash read and write operations Guennadi Liakhovetski
2008-08-31 14:58 ` Wolfgang Denk
2008-08-31 16:04 ` Guennadi Liakhovetski
2008-08-31 18:57 ` Wolfgang Denk
2008-08-31 19:45 ` Guennadi Liakhovetski
2008-08-31 19:56 ` Wolfgang Denk
2008-08-27 15:52 ` [U-Boot] [PATCH 3/6] "return" is not a function Guennadi Liakhovetski
2008-08-31 14:59 ` Wolfgang Denk
2008-08-31 16:10 ` Guennadi Liakhovetski
2008-08-31 18:57 ` Wolfgang Denk
2008-08-31 19:17 ` Guennadi Liakhovetski
2008-08-27 15:52 ` [U-Boot] [PATCH 4/6] Unify active vs. redundant environment variable naming Guennadi Liakhovetski
2008-08-31 15:04 ` Wolfgang Denk
2008-08-31 16:18 ` Guennadi Liakhovetski
2008-08-31 18:57 ` Wolfgang Denk [this message]
2008-08-31 19:27 ` Guennadi Liakhovetski
2008-08-31 19:44 ` Wolfgang Denk
2008-08-27 15:52 ` [U-Boot] [PATCH 5/6] Support environment anywhere within erase area Guennadi Liakhovetski
2008-08-31 18:57 ` Wolfgang Denk
2008-08-31 19:39 ` Guennadi Liakhovetski
2008-08-31 19:53 ` Wolfgang Denk
2008-08-27 15:52 ` [U-Boot] [PATCH 6/6] Support environment in NAND Guennadi Liakhovetski
2008-08-29 9:29 ` [U-Boot] [PATCH 6/6 v2] " Guennadi Liakhovetski
2008-08-31 18:57 ` [U-Boot] [PATCH 6/6] " Wolfgang Denk
2008-08-31 21:53 ` Guennadi Liakhovetski
2008-08-31 20:21 ` [U-Boot] [PATCH 0/6] Support NAND in fw_printenv/fw_setenv Wolfgang Denk
2008-08-31 20:37 ` Guennadi Liakhovetski
2008-08-31 20:55 ` Wolfgang Denk
2008-09-01 9:08 ` Guennadi Liakhovetski
2008-09-01 9:31 ` Guennadi Liakhovetski
2008-09-01 22:42 ` Wolfgang Denk
2008-09-01 22:41 ` Wolfgang Denk
2008-09-01 23:33 ` Guennadi Liakhovetski
2008-09-02 0:13 ` Wolfgang Denk
2008-09-02 16:00 ` Guennadi Liakhovetski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080831185730.5798F242FF@gemini.denx.de \
--to=wd@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.