Review Request 114152: [GCI] FixPicturestrip navigation loop

[Sebastian Kügler]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20131127/bea4c7c9/attachment-0001.html>