From: "Bjørn Mork" <bjorn@mork.no>
To: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Andrej Manduch <amanduch@gmail.com>,
Nicholas Krause <xerofoify@gmail.com>,
jesse.brandeburg@intel.com, bruce.w.allan@intel.com,
carolyn.wyborny@intel.com, donald.c.skidmore@intel.com,
gregory.v.rose@intel.com, matthew.vick@intel.com,
john.ronciak@intel.com, mitch.a.williams@intel.com,
linux.nics@intel.com, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function,gb_acquire_swfw_sync_i210
Date: Mon, 09 Feb 2015 10:29:35 +0100 [thread overview]
Message-ID: <87egpzqvpc.fsf@nemi.mork.no> (raw)
In-Reply-To: <1423470722.27854.11.camel@jtkirshe-mobl> (Jeff Kirsher's message of "Mon, 09 Feb 2015 00:32:02 -0800")
Jeff Kirsher <jeffrey.t.kirsher@intel.com> writes:
> If you want to see Nick's patch, feel free to view his patch on
> my queue tree:
> https://git.kernel.org/cgit/linux/kernel/git/jkirsher/queue.git/
which said:
- s32 i = 0, timeout = 200; /* FIXME: find real value to use here */
+ s32 i = 0, timeout = 200;
Comments like the one deleted by that patch do have some value in my
opinion: They document that the constant is chosen arbitrarily and is
not taken from some datasheet.
Not a big deal, but leaving such comments, even if the value never ever
changes, could save someone from trying to figure out where the heck you
got that constant. And there's noone actually misinterpreting this
comment as "something needs to be fixed here", is there? So the cost of
leaving the comment is exactly zero.
Just my .02 €. I'm not going to tell you how to maintain your driver,
of course. At least I try not to :-)
Bjørn
next prev parent reply other threads:[~2015-02-09 9:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-08 5:21 [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210 Nicholas Krause
2015-02-09 1:21 ` [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function,gb_acquire_swfw_sync_i210 Jeff Kirsher
2015-02-09 1:21 ` [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210 Jeff Kirsher
2015-02-09 1:26 ` [E1000-devel] " Jeff Kirsher
2015-02-09 1:26 ` Jeff Kirsher
2015-02-09 1:28 ` nick
2015-02-09 1:29 ` [E1000-devel] " Jeff Kirsher
2015-02-09 7:17 ` [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function,gb_acquire_swfw_sync_i210 Andrej Manduch
2015-02-09 8:32 ` Jeff Kirsher
2015-02-09 9:29 ` Bjørn Mork [this message]
2015-02-09 9:35 ` Jeff Kirsher
2015-02-09 17:03 ` [E1000-devel] [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210 Hisashi T Fujinaka
2015-02-09 17:03 ` Hisashi T Fujinaka
2015-02-09 19:35 ` Nick Krause
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=87egpzqvpc.fsf@nemi.mork.no \
--to=bjorn@mork.no \
--cc=amanduch@gmail.com \
--cc=bruce.w.allan@intel.com \
--cc=carolyn.wyborny@intel.com \
--cc=donald.c.skidmore@intel.com \
--cc=e1000-devel@lists.sourceforge.net \
--cc=gregory.v.rose@intel.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=john.ronciak@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux.nics@intel.com \
--cc=matthew.vick@intel.com \
--cc=mitch.a.williams@intel.com \
--cc=netdev@vger.kernel.org \
--cc=xerofoify@gmail.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.