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