Referencing past commits that made code redundant (was: Re: [RFC] Performance improvements)

Andrew Bartlett abartlet at samba.org
Fri Jul 13 20:37:32 UTC 2018


On Fri, 2018-07-13 at 12:56 +0200, Swen Schillig via samba-technical
wrote:
> >   The destructor was once useful. 
> >   All I'm asking you to do is mention in the commit message
> > when it stopped being useful. 
> 
> Hmm, I thought it is enough to see that it is useless now.
> Anyhow, not really sure about its usefulness in the past but I believe
> it definitely lost its "purpose" with David's patch to make
> queue_io_read reentrant.
> 
> commit e097b7f8ff1a9992de1d11474dac4969e30cd679
> Author: David Disseldorp <ddiss at suse.de>
> Date:   Sun Jul 31 03:14:54 2011 +0200
> 
>     io: Make queue_io_read() safe for reentry
> 
> >   I'm not arguing against removal of the
> > destructor.  I just don't want to have to go on archaeological dig
> > every time I look at this code.  If your commit message points to the
> > right point in the past then I know what I need to know and you've
> > saved me some work.
> 
> So am I getting you right in saying that if I'm adding a comment with
> the above reference would convince you to accept his one ?

G'Day Swen,

Perhaps you were in my Samba patterns and anti-patterns talk at
SambaXP.  If you were, you may have seen me poke fun at Samba being
'git patches as performance art'.

But it is true, Samba patches should tell a story, and should reference
history.  Including the git commit where something stopped being needed
is really helpful, because in cases like this, is it considered just as
likely that the referenced commit was faulty as that this is
intentionally unused. 

Your job as a submitter is to rebut that assumption, and in turn help
the future developer who has to work out why something went away.  (Our
git history is regularly the subject of code archaeology). 

I hope this clarifies things,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba




More information about the samba-technical mailing list