[dpdk-dev] [PATCH] devtools: check commit log fixes syntax

[Bruce Richardson]

On Wed, Jan 30, 2019 at 11:17:00AM +0000, Ferruh Yigit wrote:
> On 1/30/2019 9:58 AM, David Marchand wrote:
> > On Tue, Jan 29, 2019 at 7:07 PM Ferruh Yigit <ferruh.yigit at intel.com> wrote:
> > 
> >> On 1/29/2019 5:34 PM, David Marchand wrote:
> >>> On Tue, Jan 29, 2019 at 4:31 PM Ferruh Yigit <ferruh.yigit at intel.com>
> >> wrote:
> >>>
> >>>> Fixes line commit id length defined as 12 in fixline alias:
> >>>> fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'
> >>>>
> >>>> Check if the Fixes line commit id length matches the defined value.
> >>>>
> >>>
> >>> Can't git decide to report a longer string in case of collisions of
> >>> abbreviated id ?
> >>>
> >>> Tried this for 2 characters, and git forcefully reported 5 chars:
> >>> $ git log -1 --abbrev=2 origin/master --format='Fixes: %h (\"%s\")'
> >>> Fixes: a2f9c (\"version: 19.02-rc4\")
> >>>
> >>> I did not find any collisions with 12 characters abbreviated commitid,
> >> but
> >>> I am not sure enforcing the check on exactly 12 characters is a good idea
> >>> in the long run.
> >>
> >> Yes git can report a longer string in case of collisions, but I don't
> >> expect to
> >> have one with 12 characters.
> >>
> >> This is mainly for some cases either people use full 40 chars or small
> >> ones.
> >>
> >> Indeed in background I am (and most probably Thomas too) fixing them while
> >> merging, I thought it is better idea to integrate that into script so that
> >> developers can be aware of the syntax issue and fix it before sending.
> >>
> > 
> > I can understand you want to avoid such edits yes, and so this patch.
> > 
> > However, I think we can do one more thing.
> > The contributing guide does indicate you are supposed to run both
> > checkpatches.sh and check-git-log.sh.
> > I am pretty sure I missed this second step in the past..
> > 
> > How about calling check-git-log.sh from checkpatches.sh ?
> > check-git-log.sh does not support patch files as input, so it would need
> > support for it.
> 
> That sounds good idea to have single script to run.
> 
+1 to this