[Development] Reviewing new classes (was: Re: Qt 5.2 header diff)

Marc Mutz marc.mutz at kdab.com
Wed Nov 13 12:55:51 CET 2013


On Friday, November 08, 2013 13:59:00 Heikkinen Jani wrote:
> Hi all,
> 
> What is the situation with reviews & especially potential fixes? Is all
> needed now done and so on can we start merge on Monday morning?

Maybe I missed the mail, but there was no explict round of review of new 
classes.

I've reviewed the new classes (at least those that come with new files[1]) in 
QtCore, QtGui, and QtWidgets.

core:
  https://codereview.qt-project.org/71116
  https://codereview.qt-project.org/71117
  https://codereview.qt-project.org/71118
  https://codereview.qt-project.org/71119
  https://codereview.qt-project.org/71120
  https://codereview.qt-project.org/71129
  https://codereview.qt-project.org/71131
  https://codereview.qt-project.org/71132
  https://codereview.qt-project.org/71133
  https://codereview.qt-project.org/71134
gui:
  https://codereview.qt-project.org/71141
  https://codereview.qt-project.org/71142
  https://codereview.qt-project.org/71143

When adding new value classes, please check that they
 a) are marked Q_DECLARE_SHARED if they are Q_MOVABLE_TYPE and copyable, even
     if they are not implictly shared. The macro should be renamed to
     Q_DECLARE_VALUE_CLASS at some point :)
 b) (necessary for (a)) have an inline member-swap function
 c) have an inline move assignment operator
 d) if they use a naked pointer as d-pointer, have an inline move constructor

When adding any class, please check that ctors are marked either as explicit 
or /*implicit*/.

Thanks,
Marc

[1] I used this command to find them:
  git log --diff-filter=A --stat v5.1.1..gerrit/release -- src | grep src | \
     grep \\.h | grep -v _p\\.h | cut -d\  -f2

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions



More information about the Development mailing list