Review Request 108716: FIX Randomize playlist with Ctrl+H

[Matěj Laitl]

> On Feb. 9, 2013, 12:06 p.m., Matěj Laitl wrote:
> > src/MainWindow.cpp, line 836
> > <http://git.reviewboard.kde.org/r/108716/diff/2/?file=113315#file113315line836>
> >
> >     Amarok coding style says:
> >     
> >     connect( action, SIGNAL(triggered(bool)), this, SLOT(slotShufflePlaylist()) );
> >     
> >     (note that SIGNAL/SLOT macros are the only exception when there are no spaces around arguments)
> 
> Harsh Gupta wrote:
>     Thanks of pointing out minor details. I have fixed all the above things but in the process I some how deleted one patch file. So I tried to create patch using "git format-patch commit-tag --stdout > patchfile" but it appears that there is something wrong with the patch. Please help. Thanks again.
> 
> Matěj Laitl wrote:
>     It looks like you've uploaded the difference between the second and third patch rather than the difference between origin/master and v3 of your work. If you work in git (good), it probably means that you created extra commit for the third version of your patch. I strongly recommend you use a tool like gitk to visualize changes in your git repository. All your work on this feature should be squelched into a single patch - a comand like `git rebase -i <starting-point>` (where <starting point> is the upstream commit/branch (most probably just origin/master)) can help you edit and merge existing commits.
>     
>     `git format patch orgin/master` is the right command to export commits you have on top of the upstream code.

Also note there are nice tools to work with reviewboard from git, the package is called rbtools and contains command named post-review. A command to update this review request could be then:

post-review --guess-description --username=<your username> --parent=$(git merge-base HEAD origin/master) --open -r 108716


- Matěj


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


On Feb. 9, 2013, 8:51 p.m., Harsh Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108716/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2013, 8:51 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Added a shortcut button for shuffle with default shortcut as Ctrl + H.
> Created a slot shuffleTrack() in mainWindow.cpp which is triggered on pressing Ctrl + H
> 
> 
> This addresses bug 208061.
>     https://bugs.kde.org/show_bug.cgi?id=208061
> 
> 
> Diffs
> -----
> 
>   src/MainWindow.cpp 8587784 
>   src/MainWindow.cpp 8587784 
>   src/MainWindow.h 4b23679 
>   ChangeLog 139ffa5 
>   ChangeLog 749ed6d 
> 
> Diff: http://git.reviewboard.kde.org/r/108716/diff/
> 
> 
> Testing
> -------
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Harsh Gupta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130209/b69b9b86/attachment-0001.html>