Review Request 128811: Unit tests of seperator handling - and proper emission of signals
David Faure
faure at kde.org
Thu Sep 1 13:32:34 UTC 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128811/#review98816
-----------------------------------------------------------
The commit log sounds like you only added a few unittests ("Unit tests of A - and B"). But you actually fixed the behaviour as well, (which I know understand is that the B is about). Reword to what people care for: "fixed foo bar".
I love TDD but it shouldn't be reflected in the commit log by describing the tests first ;)
src/kdescendantsproxymodel.cpp (line 225)
<https://git.reviewboard.kde.org/r/128811/#comment66519>
(I would call it s_changedRoles so we can see in the code that it's about a static variable, but I can't remember if that's in the coding style)
src/kdescendantsproxymodel.cpp (line 235)
<https://git.reviewboard.kde.org/r/128811/#comment66518>
coding style: missing spaces after commas
- David Faure
On Sept. 1, 2016, 11:15 a.m., Sune Vuorela wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128811/
> -----------------------------------------------------------
>
> (Updated Sept. 1, 2016, 11:15 a.m.)
>
>
> Review request for KDE Frameworks, David Faure and Stephen Kelly.
>
>
> Repository: kitemmodels
>
>
> Description
> -------
>
> Verify that correct signals are emitted when the data related to separators changes.
>
>
> Diffs
> -----
>
> autotests/kdescendantsproxymodeltest.cpp 6d7cd35
> src/kdescendantsproxymodel.cpp d3aabf8
>
> Diff: https://git.reviewboard.kde.org/r/128811/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sune Vuorela
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160901/8a5eaf65/attachment.html>
More information about the Kde-frameworks-devel
mailing list