[Kde-games-devel] Review request: Bug 272512 - Move army dialog moves too many armies

[Nemanja Hirsl]

On Sunday, October 07, 2012 09:38:17 PM Del wrote:
> Thanks for the information. Seems I need a new identity, different from the 
one 
> in bugzilla. Did that. 

Yes, just use the same email address as for bugzilla.

> Seems diff's are not diffs, they need to be created with 
> post-review. I may of course be stupid, or have inadequate patience to chase 
> down the documentation, but I don't see how to use post-review. Using it 
> without any parameters on the cloned git repository of ksirk I get:
> 
> Unable to find a Review Board server for this source code tree.
> 
> Pointers are appreciated.

I haven't used post-review yet, but until you (and me as well) find out how to 
properly use it and read documentation, you can attach git diff:
1. Navigate to git repository of ksirk
2. "git diff" (w/o quotes) will print diffs to stdout. Make sure all you wanted 
to change, and nothing more is there
3. Put it in the file: "git diff > patch.diff" (w/o quotes)
4. Attach patch.diff to review

Regards,
Nemanja

> 
> Cheers,
> Del
> 
> On Sunday, October 07, 2012 03:32:19 PM Nemanja Hirsl wrote:
> > On Saturday, September 29, 2012 11:31:36 AM Del wrote:
> > > This bug has been present for years now, so I finally got around to
> > > chasing
> > > it down. It is always reproducible across distributions and KDE 
releases,
> > > and is rather critical for Ksirk. I posted the patch on the bug tracker,
> > > but I assume the mailing list is the appropriate place.
> > > 
> > > Anyway, the bug is caused by the slot SlideClose being triggered twice 
in
> > > InvasionSlilder.cpp. Hence, the number on the slider (total number of
> > > armies to move minus three) is moved twice. I removed the duplicate
> > > entry, and this seems to have fixed the bug.
> > > 
> > > Really small patch, just apply to current file InvasionSlider.cpp in git:
> > > 
> > > 179,183c179
> > > <   if (invasionType == Invasion)
> > > <   {
> > > <     connect(this,SIGNAL(finished(int)),this,SLOT(slideClose()));
> > > <   }
> > > <   else if (invasionType == Moving)
> > > ---
> > > 
> > > >   if (invasionType == Moving)
> > > 
> > > Please inform me if there is more I should do here.
> > 
> > Hi Del,
> > 
> > I believe you shoud create code review, describe behavior, provide 
detailed
> > analysis (possible side effects) and tests performed.
> > Very nice to-do list can be found in Roney's answer to me some time ago:
> > http://lists.kde.org/?l=kde-games-devel&m=134818363717242&w=2
> > with a change that git reviewboard is now ready:
> > http://lists.kde.org/?l=kde- games-devel&m=134913187316402&w=2
> > 
> > If you have any trouble following any of these steps, do not hesitate to 
ask
> > for help.
> > 
> > Regards,
> > Nemanja
> > 
> > > Cheers,
> > > Del
> > > _______________________________________________
> > > kde-games-devel mailing list
> > > kde-games-devel at kde.org
> > > https://mail.kde.org/mailman/listinfo/kde-games-devel