Received: by 2002:ac0:a594:0:0:0:0:0 with SMTP id m20-v6csp6949429imm; Sun, 20 May 2018 14:41:25 -0700 (PDT) X-Google-Smtp-Source: AB8JxZorltQOb6SxSTvcHvXegRFyugujbZelA2RylIXR5pQvQEKizEdgNw1KfZAArOF9KNbUt/re X-Received: by 2002:a17:902:a986:: with SMTP id bh6-v6mr18237263plb.245.1526852485091; Sun, 20 May 2018 14:41:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526852485; cv=none; d=google.com; s=arc-20160816; b=n0Tr4vCSroTF8cDiaFGLyzub6mdb9V+hDcwO4CdZTR9QvidzwpZam9jTPyVd+e4pYm 9TvylUxubt6j9iMojLUvVqorX5rJd17+bV+q4qMlwvnMtTe9P04DxdCRe7sgwM/TdEk3 wSA57n/XjVuw0te6KDxQzi/g/9wIoDU6ky4jKRsWNCIzQp3+M9ZtpgxtsN2xOOxyjtTU 6GCP5Pyf3sglnwrUaOxwh/v+gzA5TV6LVtZcHUxIe0wEASa/KYSY4B9RXxeDVYBz5dXQ 2OaJSPd0Zl1EDYkoLzmu+w2mPH8bH7+P4VmRiilvC7Likk44Jg6IcTEE/UbOOyjDmMWc EoDg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=oNekeLDxiO8nj0dhb/6HjBG5Bt741GxYfMt0hBc7B7Q=; b=yLuNPAdeHIwIlTJPUVsTci1edNBNHH1A3+ga2sxwAQCWBoBtYu06rSh3ByHFfYCha8 7wblL9qGdPY75MwqQTke9u6+etG3xBbGg1HHCzQ0nlsPikLMEZPqSe2oRRxS5kUuIW+x W0y/DKIBpj3QGDnKD8kloumgmbmBRpIye4k2RFPR2P9Kq9Z5fUN/pi1SBKPaHHY6XM/s Czr5t1Um1CPJF/ZePHVXDRA/3bXC1wpzY98FegClt3E3MsgyJz1WIWXFXODaIv2ALroQ srybB/hs4npwCRNz3d6G1zhXZIRIfM74kPYwlpHAax5SB9HRvBa44gMIDADZd/c2BGUV eb+Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=FesLvWWE; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c14-v6si12448484pfe.29.2018.05.20.14.41.08; Sun, 20 May 2018 14:41:25 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=FesLvWWE; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751911AbeETVk6 (ORCPT + 99 others); Sun, 20 May 2018 17:40:58 -0400 Received: from mail-ot0-f196.google.com ([74.125.82.196]:34083 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751504AbeETVk4 (ORCPT ); Sun, 20 May 2018 17:40:56 -0400 Received: by mail-ot0-f196.google.com with SMTP id i5-v6so14811754otf.1; Sun, 20 May 2018 14:40:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=oNekeLDxiO8nj0dhb/6HjBG5Bt741GxYfMt0hBc7B7Q=; b=FesLvWWEeZVFkmeNyzYh9rvG+vZQkTzMup8DT0mscFYbrDdqtNu/hYiYHU5/CUrYaF EJpUhXofbln/DipJSqSFb8zwZ6AwX3YnlhSyCyJi6jbLjrAg84qwht46i3+30wjBnBnm 6D9rxafVBkvo2CkNKWbMH2DY4eDZfKlLcr5GcUBfct5I9HFZT1kaepwPpuCKFepnYoOZ ptG3gJtTAs7tn8va+MpJyBctP9IFoQ9+g9smp7YDun67ET2UUbj9jQSJue18JJ81JBAw kO1NUjraqCpdgdUnE+YjNO8BFYPzzhSLdLus+qSsBtot0PZtldiDo+dKEctU4eO+4wPt pQFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=oNekeLDxiO8nj0dhb/6HjBG5Bt741GxYfMt0hBc7B7Q=; b=IQwdJJ7KuY01z+A0nwCJB+XSDQ4fMZy3/13H84qZx7T8u1DAKkyQ18IvtlsKZwC2Qy JqpBdUykRXyxzw0lnVLshPiEbF1SOwFcUQIKSoknHkkpQTuTVLrp9IefDy0MZDpqDpvp 3L6HPxJOTiPymaUGBYXRuL7u1ne4u5ve/4jOk3hIdzfTOxOc9C0r9mj/lOR9tj3SkiV1 jw0SujHH7MYHrHosNQGdHJE6masBWCO7eMtam/DT8KqMQLIaTfZ5pJKyk69nMC0F8uMw iIqCmvMjuqJnDLk4xArC6erp+bcxY+sNkyPqRR2LqZi11E/IjOiHGO7lnYenehR0UM65 mdaA== X-Gm-Message-State: ALKqPwfkb2CL27uVUy8B/HpU6mwikIWNcKS3BLJfS/lr9aA8OdbkP8ML 4E5oJWmqZpZNQeutAdSnj03auRvIwYX0XCx2IwSIrg== X-Received: by 2002:a9d:e43:: with SMTP id n3-v6mr11131658otd.95.1526852455310; Sun, 20 May 2018 14:40:55 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:aca:ab4b:0:0:0:0:0 with HTTP; Sun, 20 May 2018 14:40:54 -0700 (PDT) In-Reply-To: <20180520213349.GC26212@localhost.localdomain> References: <1526308035-12484-1-git-send-email-vladbu@mellanox.com> <1526308035-12484-14-git-send-email-vladbu@mellanox.com> <20180519222028.GF5488@localhost.localdomain> <20180520213349.GC26212@localhost.localdomain> From: Or Gerlitz Date: Mon, 21 May 2018 00:40:54 +0300 Message-ID: Subject: Re: [PATCH 13/14] net: sched: use unique idr insert function in unlocked actions To: Marcelo Ricardo Leitner Cc: Vlad Buslov , Linux Netdev List , David Miller , Jamal Hadi Salim , Cong Wang , Jiri Pirko , Pablo Neira Ayuso , kadlec@blackhole.kfki.hu, Florian Westphal , Alexei Starovoitov , Daniel Borkmann , Eric Dumazet , keescook@chromium.org, Linux Kernel , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, Yevgeny Kliteynik Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 21, 2018 at 12:33 AM, Marcelo Ricardo Leitner wrote: > On Mon, May 21, 2018 at 12:13:06AM +0300, Or Gerlitz wrote: >> On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner >> wrote: >> > On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote: >> >> Substitute calls to action insert function with calls to action insert >> >> unique function that warns if insertion overwrites index in idr. >> > >> > I know this patch may be gone on V2, but a general comment, please use >> > the function names themselves instead of a textualized version. I.e., >> > s/action insert unique/tcf_idr_insert_unique/ >> >> disagree. While doing reviews I found out that if I ask the developer >> to use higher >> level / descriptive language and specifically to avoid putting >> variable / function names in >> patch titles and change logs, the quality gets ++ big time, vs if the >> developer is allowed to say >> >> net/mlx5: Changed add_vovo_bobo() >> >> Added variable do_it_right to add_vovo_bobo(), now we are terribly good. > > In your example I agree that it is not helping and it is even allowing > such empty changelog, just as in the section I highlighted, the > descriptive language is also not helping IMHO. > > I had to read it 3 times to make sure I wasn't missing a modifier word > when comparing the two functions and well, it's just saying > "Substitute calls to foo function to bar function". I don't see how > the textualized version helps in this case while, at least in this > one, I would have visually recognized the function names way faster. > > Sounds like 2 bad examples for either approach. Properly written descriptive language is maybe harder to come up with (specifically for those of us who are not native english speakers, like me) but is more expressive and helpful for reviews and maintenance. Lets try to add Vlad to come up with the right higher language and not ask him to quote function and variable named.