> On March 28, 2013, 6:33 p.m., Harsh Gupta wrote: > > src/EngineController.cpp, line 793 > > <http://git.reviewboard.kde.org/r/108995/diff/2/?file=119751#file119751line793> > > > > How am I suppose to stage changes in a line ( line 791 ) in which a > > variable is first renamed and then get deleted ? Should I commit all the > > changes again in order to make two different patches ? > > Harsh Gupta wrote: > Oops... sorry for poor tagging !!! (first time :P) > Line no. is actually 791. > > Matěj Laitl wrote: > `git add -p` allows you to edit the hunks while staging, so you can use > it to "generate" changes that "annihilated". From "oldVarName" -> "" you can > create "oldVarname" -> "newVarName" -> "". (where oldVarName would be in > tree, newVarName would be in index (staged to commit) and "" would be in > working tree) > > But perhaps this is too complicated for an user new to git. You may > instead do: > 1. start a new branch starting just a commit before your changes: git > branch newbranch oldbranch^ (or master^ if you've worked directly in master) > 2. perform the exact same rename using your IDE (should be quick to do), > commit > 3. check out the working tree of oldbranch (master?) without switching to > it: git checkout oldbranch -- . > 4. `git diff` should show the "real" changes w/thout the renames; commit > 5. now you have 2 commits, yay! Ensure that both commits build. > > Harsh Gupta wrote: > Should I submit both the patches in this review request one by one ? > PS: Please ignore my latest patch. I was expecting two different patches > (one as a parent diff) but review board merged the two patches. > > Matěj Laitl wrote: > > Should I submit both the patches in this review request one by one? > > Please submit the renaming patch as new review request here. Indicate in > this review request that it depends on the renaming one. (and optionally > update it so that it applied on top of the renaming patch - but this seems > already done) > > > I was just wondering what should I write in the Changelog file? > > BUGFIXES: > * Pre-aplifier in equalizer is now only enabled if it is actually > supported; patch by Harsh Gupta. (BR 301311) > > Harsh Gupta wrote: > I have created a new review request for the renaming patch. Should I wait > for that patch to get shipped ? > > PS : Sorry for the late response. University projects kept me occupied :(
> I have created a new review request for the renaming patch. Should I wait for > that patch to get shipped? It already is. :-) > PS: Sorry for the late response. University projects kept me occupied :( No problem, glad to know you weren't hit by a bus. Actual review below. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108995/#review30021 ----------------------------------------------------------- On April 13, 2013, 4:14 p.m., Harsh Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108995/ > ----------------------------------------------------------- > > (Updated April 13, 2013, 4:14 p.m.) > > > Review request for Amarok. > > > Description > ------- > > 1. Disabled Pre-amplifier in equalizer if it is not supported by phonon. > 2. Fixed top and bottom labels of first slider. Earlier band name label was > at the top and slider value label was at the bottom for first slider. > 3. Removed an extra semicolon EqualizerDialog.h . > Note : I have made an assumption that if at all preamp is present then it > will the first element of Effect Parameter list. > > > This addresses bug 301311. > https://bugs.kde.org/show_bug.cgi?id=301311 > > > Diffs > ----- > > ChangeLog a278be3 > src/EngineController.h 5de4beb > src/EngineController.cpp 28fb256 > src/dialogs/EqualizerDialog.cpp f42a033 > src/dialogs/EqualizerDialog.ui 43b0187 > > Diff: http://git.reviewboard.kde.org/r/108995/diff/ > > > Testing > ------- > > All unit test cases passed. > Note : I have tested it with gstreamer only. Xine phonon keep crashing on my > PC. > > > File Attachments > ---------------- > > Equalizer snapshot > http://git.reviewboard.kde.org/media/uploaded/files/2013/02/17/equalizer.png > > > Thanks, > > Harsh Gupta > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel