> On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote: > > src/statusbar/CompoundProgressBar.cpp, line 46 > > <http://git.reviewboard.kde.org/r/105942/diff/1/?file=76720#file76720line46> > > > > Nitpicky: I would personally strip all the added newlines > > Ralf Engels wrote: > Actually I think that newlines are a good device to structure code. > I use it to split different functional blocks inside a function. > > Often I use them to split up between the initialization, the actual > function and the cleaning up inside a function. > That's why there is a newline between the Locker and the rest of the code.
Single line comments for those blocks (ex. //Initialization) are even better then. newlines are just newlines for anyone not aware of your intended structuring. Some structure in cpp files is actually a good thing. I usually keep it limited to having the methods in the same order as in the header file. - Bart ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105942/#review17141 ----------------------------------------------------------- On Aug. 9, 2012, 10:36 a.m., Ralf Engels wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105942/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2012, 10:36 a.m.) > > > Review request for Amarok. > > > Description > ------- > > childBarFinished was called with invalid m_progressDetailsWidget > However also the m_progressMap needs protection. The CompoundProgressBar > needs to be thread safe. > > > This addresses bug 292553. > https://bugs.kde.org/show_bug.cgi?id=292553 > > > Diffs > ----- > > src/statusbar/CompoundProgressBar.h baaab94 > src/statusbar/CompoundProgressBar.cpp 371a534 > > Diff: http://git.reviewboard.kde.org/r/105942/diff/ > > > Testing > ------- > > Change is verified by me to fix the problem. > > > Thanks, > > Ralf Engels > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
