[Development] ChangeLogs

[Thiago Macieira]

On sexta-feira, 18 de janeiro de 2013 11.49.12, Oswald Buddenhagen wrote:
> yes, and the purist in me agrees. but as both kai and eskil pointed out,
> this isn't so much additional clutter, so whatever - i kind of don't
> see the change-id lines when i browse logs anyway.

The Change-Id is not redundant information. It's new information, not present 
elsewhere.

> an additional upside of doing this while creating the commit is that one
> may be more thorough with the message and the whole commit, as one is
> forced to consider it from an additional angle. so it plays in the same
> league as "needlessly" insisting on commit atomicity and other "process
> stuff".

And that leads to another downside: what if two or more commits were necessary 
to fix a bug? Then the ChangeLog line would be referring to changes that are 
not found in that commit, which could be confusing later on.

I don't buy the "be more thorough" argument. If the commit message was written 
properly, the ChangeLog line is redundant because it's rephrasing what has 
already been said before, except that it's less detailed and is written in the 
past tense (as opposed to the imperative that most people seem to prefer).

If we're going to do this, then let's agree that it must always be the 
*second* paragraph of the commit message -- that is, the first paragraph of the 
body -- and it must go with the flow.

I'd rewrite my commit message to be:

====
Clear the current thread data for the main thread

ChangeLog: Fixed a crash that would cause the QObject constructor to crash if 
it was run during application shut down (that is, in global destructors).

A common case of QObjects being created on shutdown are qDebug or qWarning 
calls in destructors (due to QTextStream's device close notification QObject).

==41000== Invalid read of size 4
==41000==    at 0x5F01ED5: bool QBasicAtomicOps<4>::ref<int>(int&) 
(qatomic_x86.h:208)
==41000==    by 0x5F01309: QBasicAtomicInteger<int>::ref() 
(qbasicatomic.h:147)
==41000==    by 0x5F24051: QThreadData::ref() (qthread.cpp:100)
==41000==    by 0x614A984: QObject::QObject(QObject*) (qobject.cpp:681)
==41000==    by 0x605E562: QDeviceClosedNotifier::QDeviceClosedNotifier() 
(qtextstream.cpp:324)
==41000==    by 0x6057E90: 
QTextStreamPrivate::QTextStreamPrivate(QTextStream*) (qtextstream.cpp:441)
==41000==    by 0x605935D: QTextStream::QTextStream(QString*, 
QFlags<QIODevice::OpenModeFlag>) (qtextstream.cpp:1059)
==41000==    by 0x5F19B4B: QDebug::Stream::Stream(QtMsgType) (in 
/home/thiago/obj/qt/qt5/qtbase/lib/libQt5Core.so.5.0.1)
==41000==  Address 0x6ee73f0 is 0 bytes inside a block of size 152 free'd
==41000==    at 0x4A0736C: operator delete(void*) (vg_replace_malloc.c:480)
==41000==    by 0x5F240BF: QThreadData::deref() (qthread.cpp:109)
==41000==    by 0x6113F6B: QCoreApplicationData::~QCoreApplicationData() 
(qcoreapplication.cpp:268)

Change-Id: I0dba895b041fe6cf81e6f8939ca85035cd00aad1
===

Note how the change log line is a relevant part of the explanation of the 
problem, and is followed by more detail that doesn't need to be part of the 
changelog.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20130118/70c4a485/attachment.sig>