-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114152/#review44577
-----------------------------------------------------------


Please specify in the description of the patch what it does, and what's wrong 
exactly, possibly why this is wrong.

You also forgot to mention how you verified the test works and doesn't break 
anything else.

This makes it hard to judge for a reviewer.


mediaelements/imageviewer/PictureStrip.qml
<http://git.reviewboard.kde.org/r/114152/#comment31822>

    Generally, we put } else { on the same line.



mediaelements/imageviewer/PictureStrip.qml
<http://git.reviewboard.kde.org/r/114152/#comment31825>

    In order to make it easier for reviewers, stylistic changes should be 
submitted in a separate review.
    
    Imagine someone doing a git bisect in two years, and looks over this patch 
to see if it introduced a problem, and why. It should be as clear and concise 
as possible to read, so the reviewer can quickly decide if this matters, or not.
    
    It's tempting to do stylistic changes, but not doing it makes it easier for 
reviewers to evaluate the patch.



mediaelements/imageviewer/PictureStrip.qml
<http://git.reviewboard.kde.org/r/114152/#comment31823>

    on the same line, please


- Sebastian Kügler


On Nov. 27, 2013, 2:04 p.m., Egor Matirov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114152/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2013, 2:04 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> -------
> 
> Fix Picturestrip navigation loop. GCI task: 
> http://www.google-melange.com/gci/task/view/google/gci2013/5783943471169536
> 
> 
> Diffs
> -----
> 
>   mediaelements/imageviewer/PictureStrip.qml 4825f2e 
> 
> Diff: http://git.reviewboard.kde.org/r/114152/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Egor Matirov
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to