All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yang, Yi" <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Jiri Benc <jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: "dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org"
	<dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
	"netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"e@erig.me" <e@erig.me>,
	"davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org"
	<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Subject: Re: [PATCH net-next v10] openvswitch: enable NSH support
Date: Tue, 26 Sep 2017 21:52:41 +0800	[thread overview]
Message-ID: <20170926135240.GA88616@cran64.bj.intel.com> (raw)
In-Reply-To: <20170926130534.170270e3@griffin>

On Tue, Sep 26, 2017 at 07:05:34PM +0800, Jiri Benc wrote:
> On Tue, 26 Sep 2017 12:47:16 +0800, Yi Yang wrote:
> > +	return ((ret != 0) ? false : true);

Jiri, I appriciate your review very carefully and professionally from my
heart for 10 versions, that is really very very big effort.

I carefully thought this comment:

"""
> +	return ((ret != 0) ? false : true);

Too little coffee or too late in the night, right? ;-)
"""

But I don't think this is a problematic line from my understanding,
because validate_nsh is declared to return bool. I had thought you were
saying "it was late, it was time for you to leave office, so no more
comments", this is my readout :-)

So the best comment you can give out here is one line you prefer :-)

I'm not a kernel developer full time, I'm adapting to several coding
style at the time in kernel, OVS and Opendaylight.

> 
> I'm not going to review this version but this caught my eye - I pointed
> out this silly construct in my review of v9. I can understand that
> working late in the night and rewriting the code back and forth, one
> could end up with such construct and overlook it at the final
> self-review before submission. Happens to everyone.
> 
> But leaving this after a review pointed it out means you're not paying
> enough attention to your work. Even the fact that you sent v10 so
> shortly after v9 means you did not spend the needed time on reflecting
> on the review and that you did not properly test the new version. And
> I told you exactly this before.
> 
> Honestly, I'm starting to be tired with reviewing your patch again and
> again and pointing out silly mistakes like this one and repeating basic
> things to you again and again.

I did test it in my sfc test environment, I don't see any warning, error
during build and runtime, it can work well.

Sorry if missing your comments, that is understanding issue in most
cases but not I don't take your comments seriously. You know English
isn't my native language, it is a big challenge for me to understand
your comments very well. Neverthless, code is our common language, I can
understand code better than your descriptive words :-)

> 
>  Jiri

  reply	other threads:[~2017-09-26 13:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-26  4:47 [PATCH net-next v10] openvswitch: enable NSH support Yi Yang
2017-09-26 10:50 ` Jiri Benc
     [not found] ` <1506401236-5716-1-git-send-email-yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-09-26 11:05   ` Jiri Benc
2017-09-26 13:52     ` Yang, Yi [this message]
2017-09-26 14:42       ` Jiri Benc
2017-09-27  0:52         ` Yang, Yi

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=20170926135240.GA88616@cran64.bj.intel.com \
    --to=yi.y.yang-ral2jqcrhueavxtiumwx3w@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org \
    --cc=e@erig.me \
    --cc=jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /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.