Re: [dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2017-01-16 19:19, Yuanhan Liu:
> 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 <thomas.monjalon@6wind.com>
> > > > 
> > > > 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

OK I understand your long term goal.
I suggest to move forward by explaining this need in the contribution guide.