public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: BlueZ development <bluez-devel@lists.sourceforge.net>
Subject: Re: [Bluez-devel] [PATCH] Updated sendto
Date: Mon, 25 Feb 2008 21:36:43 +0100	[thread overview]
Message-ID: <1203971803.28528.91.camel@californication> (raw)
In-Reply-To: <1203936997.2754.198.camel@cookie.hadess.net>

Hi Bastien,

> > 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.

do you have an updated patch or do I have to do it by myself.

> > 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.

What I currently got from code review is that we simply call
gtk_main_quit() instead of having a little bit better structured code.
Just stopping the mainloop at more then 1 or 2 places doesn't seem right
to me. Feel free to convince me otherwise.

> >  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?

That behavior is fine with me. However that was not what I said. So I
expect two gtk_main_quit() inside the code. One for SIGTERM and one when
we are actually done with the transfer.

Regards

Marcel



-------------------------------------------------------------------------
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/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

  reply	other threads:[~2008-02-25 20:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-17 10:43 [Bluez-devel] [PATCH] Allow multiple files to be sent Bastien Nocera
2008-01-18 13:43 ` Bastien Nocera
2008-02-01 13:14   ` Bastien Nocera
2008-02-01 16:08     ` Bastien Nocera
2008-02-01 16:20       ` Bastien Nocera
2008-02-06 12:18         ` [Bluez-devel] [PATCH] Updated sendto Bastien Nocera
2008-02-06 13:08           ` Bastien Nocera
2008-02-07  0:59             ` Bastien Nocera
2008-02-23  1:30               ` Bastien Nocera
2008-02-24 18:29                 ` Bastien Nocera
2008-02-25  2:21                   ` Marcel Holtmann
2008-02-25 10:56                     ` Bastien Nocera
2008-02-25 20:36                       ` Marcel Holtmann [this message]
2008-02-26  1:18                         ` Bastien Nocera
2008-02-26  1:21                           ` Marcel Holtmann
2008-02-27  1:28                             ` Bastien Nocera

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1203971803.28528.91.camel@californication \
    --to=marcel@holtmann.org \
    --cc=bluez-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox