Hi Hendrik,

On Sun, Jun 5, 2011 at 11:55 AM, Hendrik Sattler
<p...@hendrik-sattler.de> wrote:
> Am Freitag, 3. Juni 2011, 09:24:21 schrieb Luiz Augusto von Dentz:
>> Hi Hendrik,
>>
>> On Thu, Jun 2, 2011 at 5:42 PM, Luiz Augusto von Dentz
>>
>> <luiz.de...@gmail.com> wrote:
>> > Hi Hendrik,
>> >
>> > On Wed, Jun 1, 2011 at 2:11 PM, Hendrik Sattler <p...@hendrik-sattler.de>
> wrote:
>> >> Zitat von Luiz Augusto von Dentz <luiz.de...@gmail.com>:
>> >>> Hi Hendrik,
>> >>>
>> >>> On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler
>> >>>
>> >>> <p...@hendrik-sattler.de> wrote:
>> >>>> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz:
>> >>>>> > Note that those are build warnings, not build errors (except when
>> >>>>> > using maintainer mode).
>> >>>>>
>> >>>>> Which is the whole point to force fixing and not just ignore them.
>> >>>>
>> >>>> Your commit message is wrong, that's what I meant!
>> >>>
>> >>> I did rebase your changes on top of my build fixes here:
>> >>> git://gitorious.org/~vudentz/openobex/vudentz-clone.git
>> >>>
>> >>> If you are ok, I will resend my 2 build fixes again. I also did a
>> >>> quick review on the other changes and noticed at least one use of c++
>> >>> comment style '//'
>> >>
>> >> Citing from the ISO 9899-1999 (C99) standard:
>> >> "6.4.9 Comments
>> >> 1 Except within a character constant, a string literal, or a comment,
>> >> the characters /* introduce a comment. The contents of such a comment
>> >> are examined only to identify multibyte characters and to find the
>> >> characters */ that terminate it.
>> >>
>> >> 2 Except within a character constant, a string literal, or a comment,
>> >> the characters // introduce a comment that includes all multibyte
>> >> characters up to, but not including, the next new-line character. The
>> >> contents of such a comment are examined only to identify multibyte
>> >> characters and to find the terminating new-line character."
>> >>
>> >> It's the year 2011, so using
>> >> /* some comment */
>> >> or
>> >> // some comment
>> >> are both absolutely valid C. Johan also mentioned this and I gave him
>> >> the same response.
>> >
>> > You are right that it is in fact a valid comment format as per C99,
>> > but you should have know that we mostly follow linux kernel code style
>> > (http://www.kernel.org/doc/Documentation/CodingStyle), which say:
>> >
>> > Linux style for comments is the C89 "/* ... */" style.
>> > Don't use C99-style "// ..." comments.
>> >
>> > Coding style is a matter of personal preference, please lets not start
>> > arguing about it now, besides you didn't really responded if the
>> > patches do look ok to you so I will assume you are and will resent
>> > them in a moment so we don't waste more time with this.
>
> "linux kernel code style" has lots of non-sense beyond reasoning, not just
> comments. It is written nowhere that this style is used with OpenOBEX, neither
> was it discussed ever. And yes, I reject this coding style in many parts.

Well I thought OpenOBEX follows BlueZ in this... anyway we should be
following the some coding style, if it is not defined we should talk
to Johan and get this sort out.

>> >>> and missing note about action command support on
>> >>> ChangeLog, can you fix those?
>> >>
>> >> Yes.
>
> I _was_ already mentioned but I changed it:
> -       Add new definitions from OBEX 1.3 errata
> +       Add new definitions from OBEX 1.3 errata: ACTION command and headers

Looks good to me.

>> >>> And please lets be more careful with
>> >>> codestyle, there are some files with space mixed with tabs and a lot
>> >>> of empty likes with tabs.
>> >>
>> >> I'll check that. The long week-end in Germany should give me enough time
>> >> :)
>> >>
>> >>> If those issues are fixed we can do 2.0 release.
>> >>
>> >> Where?
>> >
>> > In your tree, also please send them as a proper git patch (git
>> > format-path + git send-email) so we got them in the mail archive
>> > properly, if the series is too big e.g 10 > commits if possible break
>> > them in sub series waiting each series to be properly applied upstream
>> > before sending the next. I will try to help Johan to review your
>> > changes like sometimes I do for obexd.
>>
>> In case you meant where we will do the release, I was talking about
>> tag 2.0 upstream on git.kernel.org repository and create the tarball
>> as usual.
>
> Hint: "git send-email" itself is enough.
>
> I changed the patch you edited and reordered the patch set. I will send them.
> For the "trailing spaces" issue: how do I tell git to give me a warning for a
> _present_ patch?

No idea, I use vi with some extra checks for trailing spaces and
things like that, but Im quite sure you can do this with git.

Btw, your new series looks good enough to be applied.

-- 
Luiz Augusto von Dentz
Computer Engineer

------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today. 
http://p.sf.net/sfu/quest-dev2dev2 
_______________________________________________
Openobex-users mailing list
Openobex-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/openobex-users

Reply via email to