Review Request: Fix on Bug 295106 (The reset button in kcm_ktp_chat_apparence is not working)

[George Kiagiadakis]

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


Ok, let's start from the basics: Use of the review board.
1) You should not attach the .diff file. Instead, you should upload it where it says "Diff" on the new review request page.
2) Diff files must be either generated with "git diff" (if you are using git) or with "diff -uprN oldfile newfile". The current format that you uploaded is wrong.
3) You do not need to attach the whole file. The diff, if uploaded properly, is enough.
4) Review persons/groups: Just the "telepathy" group is enough.
5) Bugs field: You should enter only bug numbers there, not any words.

Regarding the diff:
1) It is incomplete, it does not show changes in the header. Since you are doing diffs manually, extract the entire tarball contents in a new directory and compare the two directories with "diff -uprN originaldir modifieddir". Git helps of course...
2) The load function is definitely not correct. At least all the connect() statements should not be executed twice, so they should only be in the constructor.

This is all for now.

- George Kiagiadakis


On July 20, 2012, 10:46 p.m., Nick Lou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105642/
> -----------------------------------------------------------
> 
> (Updated July 20, 2012, 10:46 p.m.)
> 
> 
> Review request for Telepathy, Daniele Elmo Domenichelli and George Kiagiadakis.
> 
> 
> Description
> -------
> 
> Kde-telepathy 0.4.0, text-ui.
> Bug 295106 (The reset button in kcm_ktp_chat_apparence is not working)
> I fixed this bug, it seems working properly to me now.Please note that default button had the same problem, fixed that too.
> 
> I am sending the diff and the whole file.
> 
> 
> This addresses bugs 295106, Bug, button, in, kcm_ktp_chat_apparence, not_working, and reset.
>     http://bugs.kde.org/show_bug.cgi?id=295106
>     http://bugs.kde.org/show_bug.cgi?id=Bug
>     http://bugs.kde.org/show_bug.cgi?id=button
>     http://bugs.kde.org/show_bug.cgi?id=in
>     http://bugs.kde.org/show_bug.cgi?id=kcm_ktp_chat_apparence
>     http://bugs.kde.org/show_bug.cgi?id=not_working
>     http://bugs.kde.org/show_bug.cgi?id=reset
> 
> 
> Diffs
> -----
> 
> 
> Diff: http://git.reviewboard.kde.org/r/105642/diff/
> 
> 
> Testing
> -------
> 
> On theme config settings:
> 1)Applying a theme.
> 2)Pressing default button //action: setting options to default values.
> 3)Pressing reset button //action:setting options to applied values
> 4)Repeating this procedure several times.
> 
> 
> Thanks,
> 
> Nick Lou
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120722/05f155bf/attachment.html>