All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
	Patrick Uiterwijk <patrick@puiterwijk.org>
Cc: davem@davemloft.net, linux@arm.linux.org.uk, andrew@lunn.ch,
	netdev@vger.kernel.org, Dennis Gilmore <dennis@ausil.us>,
	Peter Robinson <pbrobinson@gmail.com>
Subject: Re: [PATCH 1/2] net: dsa: mv88e6xxx: Introduce _mv88e6xxx_phy_page_{read,write}
Date: Sat, 26 Mar 2016 13:57:00 -0700	[thread overview]
Message-ID: <56F6F79C.2030102@roeck-us.net> (raw)
In-Reply-To: <87vb49gkdo.fsf@ketchup.mtl.sfl>

On 03/26/2016 11:32 AM, Vivien Didelot wrote:
> Hi Guenter,
>
> Guenter Roeck <linux@roeck-us.net> writes:
>
>>>> Is there some good reason for changing the name of those labels ?
>>>
>>> Vivien suggested to rename this since it makes more clear that this write is
>>> meant to return to page 0 to make sure that phylib doesn't get confused
>>> about the currently active page.
>>>
>>
>> And "clear:" accomplishes that ? I would not have guessed.
>> Wonder if anyone else does. I would have used a comment.
>> 	/* Try to return to page 0 even after an error */
>> or something like that.
>
> "error" definitely doesn't make sense, especially in case of success. If
> one has a better suggestion that "clear" for the label, I don't really
> mind.

Sounds like POV to me. I don't like changing label names, because someone
else may come the next day and change it again. At the end, one ends up
in a label name war. It also makes patches look more complicated than
necessary, and it _is_ an unrelated change. I don't understand the
problem with adding a comment, and using a label name in place of a
comment seems odd to me.

Anyway, this has all become philosophical, meaning I'll stay out of it.
Pick whatever you want ...

Cheers,
Guenter

  parent reply	other threads:[~2016-03-26 20:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-26  0:10 [PATCH 1/2] net: dsa: mv88e6xxx: Introduce _mv88e6xxx_phy_page_{read,write} Patrick Uiterwijk
2016-03-26  0:10 ` [PATCH 2/2] net: dsa: mv88e6xxx: Clear the PDOWN bit on setup Patrick Uiterwijk
2016-03-26 18:36   ` Vivien Didelot
2016-03-26  1:45 ` [PATCH 1/2] net: dsa: mv88e6xxx: Introduce _mv88e6xxx_phy_page_{read,write} Guenter Roeck
2016-03-26  1:58   ` Patrick Uiterwijk
2016-03-26  2:42     ` Guenter Roeck
2016-03-26 18:32       ` Vivien Didelot
2016-03-26 19:18         ` Andrew Lunn
2016-03-26 19:37           ` Vivien Didelot
2016-03-26 19:46             ` Andrew Lunn
2016-03-26 20:15               ` Vivien Didelot
2016-03-26 20:57         ` Guenter Roeck [this message]
2016-03-26 22:58           ` Vivien Didelot

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=56F6F79C.2030102@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dennis@ausil.us \
    --cc=linux@arm.linux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=patrick@puiterwijk.org \
    --cc=pbrobinson@gmail.com \
    --cc=vivien.didelot@savoirfairelinux.com \
    /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.