From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id 90B0D106A for ; Wed, 18 Jan 2017 09:32:25 +0100 (CET) Received: by mail-wm0-f44.google.com with SMTP id f73so45276238wmf.1 for ; Wed, 18 Jan 2017 00:32:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=5RvW8th5h7h+ETDdHpArVxe3l2PGORAtPIT1zV2R46o=; b=XPOZNq1ruscgAH8uCa751417Zvz2/D//hrPK8lubwc+3A2jgTXqwbZ6LttkM2s3dvd t9iFHgXlvWzaL89P0ixR74b4MFFm1zAw3y9HzSoBUoUwLlotlQh3g7NaLQSVHaXD3Xbu sDYXooNr2IC/pGS9ZpEtkL1XDHu/5s9sslHNlggrtrHqywwaa8+J3vyVi8XD5WtODbye YqkRiSggpb943n8ypR74GGxTRzXPrmtZLInQUm8t4y3g+rFhh8atH+jxOGTEULTzvS8L x5WEHGbvwKK29aM8ZcGwRURNQZJTwRcR1HJ66BV3yZ4muemv6ioqRl9WsXIgE0mGqWx4 S8/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=5RvW8th5h7h+ETDdHpArVxe3l2PGORAtPIT1zV2R46o=; b=pFKcaHkc7mo2xhfMa62OIgnr3zqvnaLHWBqxyZUHzI+eCVsbp18m4wXAYLQHxX3qXr inDWslU/6EkNeQurRwnhuRjZotZc/bNBNQC+kK7H4rZJ46aP1Ppc+Tmd/rUX6XRIUu95 shdXpTlTToRvB0tz8dB5QWO41YoXXClLEw83BGas0FlQ9HFe3SoqrmpEJgMg0qgkU+dW XtB7rWr0GXhZV38gpWu9GeaEtVgtBsOF2UKZvJ7huPsxHsFwctToT042bByLfaoedIl9 0KTKcbyAzc8rBetvTudDmIVmlzqlqy8+6D6un84GadjPqxI/NYafaBwrHx1RDrL4/0rp 9MBA== X-Gm-Message-State: AIkVDXKsbrZoFDxhFAV0pWNFKt3nwzXEScq6CzbLAYzlS7qOBcznJoUbPKPkXRQny7yAmaHU X-Received: by 10.28.101.133 with SMTP id z127mr20209380wmb.25.1484728345178; Wed, 18 Jan 2017 00:32:25 -0800 (PST) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id g197sm43692661wmd.15.2017.01.18.00.32.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Jan 2017 00:32:24 -0800 (PST) From: Thomas Monjalon To: Yuanhan Liu Cc: Ferruh Yigit , dev@dpdk.org, pablo.de.lara.guarch@intel.com, bruce.richardson@intel.com Date: Wed, 18 Jan 2017 09:32:22 +0100 Message-ID: <21531262.6kCXYB22gk@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20170118044150.GN10293@yliu-dev.sh.intel.com> References: <1484664872-26859-1-git-send-email-thomas.monjalon@6wind.com> <1860075.ojoIIAvjcn@xps13> <20170118044150.GN10293@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Jan 2017 08:32:25 -0000 2017-01-18 12:41, Yuanhan Liu: > On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: > > 2017-01-17 18:15, Ferruh Yigit: > > > On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > > > > The tag "Cc: stable@dpdk.org" must be set when the commit must be > > > > backported to a stable branch. > > > > > > > > It must be located just below the "Fixes:" tag (without blank line) > > > > and followed by a blank line, separated from SoB and review tags below. > > > > > > I am OK to keep it if it will help stable tree maintenance, but I still > > > not clear about why we need this. > > > > > > If a patch is a fix, it should already have "Fixes:" line, so this can > > > be used to parse git history. > > Same answer (as I have already replied to you in another email): not all fix > patches should be picked to stable branch. (I gave some examples below) > > > That's a question for Yuanhan. My comments below: > > > > Some fixes are not candidate for the stable branch because the bug was > > not in the previous release. These fixes are filtered out by the script > > devtools/git-log-fixes.sh > > Some fixes are not so important and we can decide they do not fit in > > the stable branch. > > Yes, I see no reason to pick patches that fix a typo, a comment issue, > a coding style issue, or even, a hypothetic bug. > > Usually, it should be a patch that fixes a solid bug, like a crash, or > something like "it behaves abnormally without the fix". That's the basic > rule. Upon that, I think we could lower the bar a bit case by case. For > example, I picked a doc update to v16.07.2: > > commit 92b70d21ee29bad92766699d0b45f579a2ff9adc > Author: Jingjing Wu > Date: Fri Sep 30 14:46:23 2016 +0800 > > doc: add limitations for i40e PMD > > [ upstream commit 972cc03ac4e30a7df8f734a77021bb15d0419b55 ] > > This patch adds "Limitations or Known issues" section for > i40e PMD, including two items: > 1. MPLS packet classification on X710/XL710 > 2. 16 Byte Descriptor cannot be used on DPDK VF > 3. Link down with i40e kernel driver after DPDK application exist > > Signed-off-by: Jingjing Wu > Acked-by: John McNamara > > It adds some limitations to the code introduced in last release: I think > it's an important note the user might want to know if he sticks to that > stable release. > > Talking about that, I think this patch makes abuse of the "Cc: stable" > tag if a developer has to (or must) add such tag for a fix patch, no > matter what it fixes. > > > Who make this decision? Relying on this Cc tag would > > mean the committers decide which patch to backport. > > Ideally, it's the developer to make such decision: he knows the best what > he is trying to fix, he also knows which version is vulnerable. > > But that's not something a new contributor might be aware of, and that's > what the maintainer (not the committer) could be helpful here: tell him > it's a candidate for a stable release and guide him on howto. > > The reason I want to stress the point of "the maintainer but not the > committer" is, normally, the maintainer knows the code better. > > > > If patch is a feature, as far as I know still can go to stable tree, but > > > for this case stable tree maintainer decides this, and author putting > > > "Cc: stable@dpdk.org" tag not so useful. Author can put this tag just > > > for recommendation, but if so why we are saving this into git history? > > > > No feature should be backported. > > > > > Initially this was to be sure fixes CC'ed to stable mail list, now > > > meaning is changing I guess. For the case author already cc'ed the > > > stable tree but not put Cc: tag into git commit, should committer add > > > this explicitly or ask from author a new version of the patch? > > > > Yuanhan was suggesting that the committer can do it if an author forget. > > Yes, it's just part of the committer job, say, adding Tested-by, > Reviewed-by, that kind of stuff. So it should be stressed in the contribution guide that it is the responsibility of the author and the maintainer to put this Cc: tag. > > > Last thing, if this tag will remain in the commit log, is this only for > > > stable tree, or any "Cc: " can stay in the history? > > > > I do not see the benefit of keeping other Cc in the history. > > Again, I really don't know why you bother to remove it, manually, commit > by commit. What's the gain here? It just adds more burden to a committer. > Honestly, I never do that. I had the personal feeling that if the Cc: person is not in SoB, Ack or Review tags, it means he's not interested in this patch. After thinking more, I was probably wrong. > It actually has benefits. Keeping the cc tags allows us to cc them when > we find a bug in that patch and make a fix patch later: they are likely > to want to know any follow updates on that original patch. > > It also helps when I send out a stable patch review: I send a copy to > all guys on the Cc list, on the Ack/Review, SoB list. I think they might > also want to know that patch is a candidate for a specific branch. He may > even give some comments, something like "if you pick this one, you might > want to pick another one", or "why bother to port it to a stable release", > that kind of thing. OK Yuanhan, thanks for the explanations. I will send a v2 to relax the constraint of blank line below Fixes: tag, and change the warning when Cc: tag is missing. Or remove the warning? What do you think of: "Reminder: is it a candidate for stable@dpdk.org backport?"