> On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote: > > Looks good, just a few small things. Could you also please describe the > > steps that lead to the crash in the commit msg?
Really? It's in the bug report pretty clearly. No need to copy it in my opinion. > On Aug. 9, 2012, 10:45 a.m., Matěj Laitl wrote: > > src/statusbar/CompoundProgressBar.h, line 32 > > <http://git.reviewboard.kde.org/r/105942/diff/1/?file=76719#file76719line32> > > > > Please document the fact that this class is now completely thread-safe. > > (or, if not, document that it is reentrant and some methods are > > thread-safe, then document every thread-safe method) I have no idea if it's now completely thread-save. I just fixed the most obvious problems. > 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 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. - Ralf ----------------------------------------------------------- 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
