Re: [PATCH v4] ethdev: fix MAC address replay

From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>, Igor Ryzhov <iryzhov@nfware.com>
Cc: Steve Shin <jonshin@cisco.com>, dev@dpdk.org
Subject: Re: [PATCH v4] ethdev: fix MAC address replay
Date: Wed, 25 Jan 2017 11:25:30 +0100	[thread overview]
Message-ID: <1679033.GODGAxT5Vg@xps13> (raw)
In-Reply-To: <5cd19f6a-311c-b0fe-5d6c-ee757ac2e86e@intel.com>

2017-01-24 13:21, Ferruh Yigit:
> On 1/24/2017 10:09 AM, Igor Ryzhov wrote:
> > Thank you Steve.
> > 
> 
> > I never did it before and I don't know if I have rights for that, but:
> > 
> > Acked-by: Igor Ryzhov <iryzhov@nfware.com <mailto:iryzhov@nfware.com>>
> 
> Unrelated to the patch itself, but since it has been mentioned, let me
> share what I know, I believe Thomas or others will correct me if I am wrong:
> 
> - Everyone can Ack.
>   And this is useful information for maintainers, so it is something
> good when more people review and ack. Please do.
> 
> - Multiple ack or review is better.
> 
> - But each Ack does not have same weight, maintainer decides on this
> weight, based on contribution of the person who ack'ed.
> 
> - There is slight difference between Acked-by and Reviewed-by:
> 
> -- Acked-by: Kind of asking for patch to be applied, saying this patch
> is good and please get it.
> 
> -- Reviewed-by: Saying I have done the review at my best and patch looks
> good to me.
> 
> Acked-by has slightly more responsibility than Reviewed-by.
> 
> If you are not maintainer of that field, and not have strong opinion
> about that patch to be merged, it is possible to prefer Reviewed-by
> against Acked-by.
> 
> But overall both are good, and definitely better than not saying
> anything at all.

We should definitely better document these tags.

My view is that Reviewed-by is stronger because it says you really checked
the patch.
Acked-by means you agree with the intent and you trust the author.
Any of these tags will be stronger if it is delivered by a maintainer.

As conclusion, here you should stress you took the review job with a
Reviewed-by tag.
A maintainer is more inclined to use the Acked-by tag, even if he does
a review.

As the maintainer of ethdev, I thank you to take the review job so I won't
have to wonder which kind of regression could be in the patch.
I will just check the intent and will rely on your Reviewed-by.