Re: [PATCH] Updated sendto

On Mon, 2008-02-25 at 03:21 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
> >>>
> >>> And one that works with obex-data-server 0.1
> >>
> >> Updated against current CVS, and fix a memleak when getting the  
> >> adapters
> >> list. New main.c file attached as well, for easier review.
> >
> > Updated against current CVS.
> 
> I've been through the main.c as whole and the actual patch. As you  
> saw, I took the dialogs out of it since that made sense and was a sane  
> way to do it. I mentioned that I want small patches and if they are  
> small and logical, I apply them most of the times immediately. Smaller  
> patches are easier to review. I am not taking a big chunk blindly.
> 
> So current patch is not acceptable. It is actually bad. So first  
> action must be to remove all these useless comments. An example is this:
> 
> +       /* Go into main loop */
>          gtk_main();
> 
> Put comments where the code is unclear and not were everybody knows  
> what it is doing. This is a perfect example of wrongly commenting code.

Done locally.

> Second of all, I am unhappy with all this usage of gtk_main_quit() in  
> various functions. Can we not just structure the code a lot more  
> cleaner to avoid multiple calls of it.

I don't understand what you mean there.

>  Besides the signal handling, I  
> would expect one extra call in case we automatically wanna close the  
> progress dialog.

Why? If there's no errors, why would you want to see it's finished? What
information would be in the dialogue that could be useful?




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/