From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id DA971106A for ; Mon, 16 Jan 2017 12:16:50 +0100 (CET) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP; 16 Jan 2017 03:16:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,238,1477983600"; d="scan'208";a="54400554" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga006.fm.intel.com with ESMTP; 16 Jan 2017 03:16:49 -0800 Date: Mon, 16 Jan 2017 19:19:04 +0800 From: Yuanhan Liu To: Thomas Monjalon Cc: Ferruh Yigit , dev@dpdk.org Message-ID: <20170116111904.GC10293@yliu-dev.sh.intel.com> References: <1479768194-6255-1-git-send-email-thomas.monjalon@6wind.com> <1b249c77-5685-8492-6f6b-3478a2731267@intel.com> <20170116095152.GQ9770@yliu-dev.sh.intel.com> <1635933.WUEFBCc6hf@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1635933.WUEFBCc6hf@xps13> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit 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: Mon, 16 Jan 2017 11:16:51 -0000 On Mon, Jan 16, 2017 at 11:37:34AM +0100, Thomas Monjalon wrote: > 2017-01-16 17:51, Yuanhan Liu: > > On Wed, Nov 30, 2016 at 02:54:14PM +0000, Ferruh Yigit wrote: > > > On 11/21/2016 10:43 PM, Thomas Monjalon wrote: > > > > Add a check for commits fixing a released bug. > > > > Such commits are found thanks to scripts/git-log-fixes.sh. > > > > They must be sent CC: stable@dpdk.org. > > > > In order to avoid forgetting CC, this mail header can be written > > > > in the git commit message. > > > > > > > > Signed-off-by: Thomas Monjalon > > > > > > I think this is useful, thanks for the patch. > > > > Yes, it is. Thanks! (Sorry for late reply; hope it's not too late). > > > > > > +[ -z "$bad" ] || printf "Should CC: stable@dpdk.org\n$bad\n" > > > > > > This is good for developer, but since "CC: xx" tags removed when patch > > > applied, > > > > Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And > > I will talk why this tag should be kept, as a stable tree maintainer. > > The drawback of keeping CC: in the mainline tree would be to give the wrong > feeling that only the commits having that tag are in the stable tree. Idealy, if a patch should be in a stable tree, there should be a "cc: stable" tag. And I don't think we are pretty far way from the idea case: as stated, I already saw people are getting used to do that. OTOH, why do we care how the commit log looks like in git history? Why do why bother to ask "hey, why this commit log has this tag while that one doesn't"? > > The policy I would expect is, leave this tag as it is, I then will apply > > all of them to a stable branch: I will no longer do the picking job. Instead, > > I may just need handle those can't apply cleanly and ask the author to > > do backport. > > > > It would be do-able now, as I saw a lot of people are getting used to add > > such tag. And even not, I saw those kind committers do that for them. > > The kind committers were adding CC: in their email reply. > You would like we add CC: also in the git tree before committing, right? Yes. I don't think that's too much effort, considering commiters normally do some minors edits while apply, such as addding Reviewed|Acked|Xxx-by, etc. > > It put another responsibility on committers. When the stable tag is forgotten > by the author, it is easy to forget adding it in the commit. And when it is > pushed, it is too late. But we can still fix this neglecting by sending an > email to stable@dpdk.org. Yes, that's why we have LTS release. > That's why I think the most reliable source of explicit tagging is the stable > mailing list, not the mainline git tree. There are too many patches there (in the stable mailing list), you never know which are applied and which are not, especially when commiters are used to change the commit subject a bit. > > Besides, if there is already an explicit way, why should we stick on the > > implicit way? > > Because the explicit way is not 100% proof. Yes, but I think it will be close to "100%" as time moves forward, when all people are really used to add "cc: stable" tag, when there are just few need to be added manually by the kind committers, when people know that the chance your patch will not be in a stable release will be MUCH higher if you don't do that. > I think we still need to check manually to avoid missing too many fixes. I agree. It's un-avoidable, especially in the first stage like where we are now. But my purpose is to create some solid working styles, so that it would be eaiser and convenient for everyone who wants to take such role in future. That said, I will still look the git history, to do some extra manual check. I may still use the script (git-log-fixes.sh) for a while. But the long goal is to get rid of it, because there are so many things you have to check mannually with that. > The other way (that you are advocating) means that we accept to miss > some fixes and it will enforce the community to become better and better to > manage that. > I can agree to your way of thinking. I just want to make it 100% clear to > everyone and read the feedbacks. > > My conclusion: the work of stable maintainer should be simpler and better > automated. If we miss some fixes because of the automated process, it means > we must be better in this process :) > The most reliable source of trust in this automated process should be the > list of patches appearing on the stable mailing list at any time (on first > send or after it has been merged in the mainline). As stated, it's really hard: despite there are many versions, the biggest difficult is commiters like to change the subject a bit. For example, here is one work from myslef :/ [PATCH] vhost: Introduce vhost-user's REPLY_ACK feature That's the one you see in the mailing list, and following is what you will see in the git (after my twist): vhost: introduce reply ack feature --yliu