Open Source Telephony
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: ofono@ofono.org
Subject: RE: [PATCH 2/4] Handle the conversion failure when parsing item
Date: Tue, 16 Mar 2010 21:44:43 -0700	[thread overview]
Message-ID: <1268801083.2700.40.camel@localhost.localdomain> (raw)
In-Reply-To: <CE761E84DADF2947A4AF22FB8D97A4731C76B5B7@shsmsx501.ccr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]

Hi Yang,

> >> -	item->text = sim_string_to_utf8(data+1, len-1);
> >> +	utf8 = sim_string_to_utf8(data+1, len-1);
> >
> >I applied the patch and fixed up the arithmetic operator style issue.  Please
> >make sure you follow this convention from now on.
> 
> Thank you, and I will pay attention to this in future. 
> 
> I'm always wondering what's the complete coding style we should follow. Can we have some document to describe them? I have collected some of them:
> 1. Check the patch using checkpatch.pl (checkpatch.pl --no-tree patch_name). In theory, you need to clean up all the warnings and errors except one "ERROR: Missing Signed-off-by: line(s)". However, sometimes, the warning of exceeding maximum characters for a line (80 characters) can be ignored.
> 2. There should be a blank line before if statement, unless it is nested and not preceded by an expression or variable declaration.
> 3. There should be space before and after operator.
> 4. It's better not to have a complicated statement for if. You may judge its contrary condition and return | break | continue ASAP.
> 5. Better to use abbreviation, rather than full name, to name a variable, function, struct, etc.
> Any others?

general rule is to look at the already existing code. We might need
checkpatch.pl specific for BlueZ, ConnMan and oFono. There are cases
where some of the style is different from kernel code. However it is not
that major.

We all make mistakes and sometimes think to complicated. That is why we
do reviews and enforce the coding style. Just keep an eye on the simple
mistakes like the one above. They are pretty obvious ;)

Regards

Marcel



  reply	other threads:[~2010-03-17  4:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16 10:01 [PATCH 1/4] Fix the structure of stk_subaddress Yang Gu
2010-03-16 10:01 ` [PATCH 2/4] Handle the conversion failure when parsing item Yang Gu
2010-03-16 10:01   ` [PATCH 3/4] Add parser for file list objects Yang Gu
2010-03-16 10:01     ` [PATCH 4/4] Add parser for location information objects Yang Gu
2010-03-16 18:46       ` Marcel Holtmann
2010-03-16 18:44     ` [PATCH 3/4] Add parser for file list objects Marcel Holtmann
2010-03-16 20:01     ` Denis Kenzior
2010-03-16 18:36   ` [PATCH 2/4] Handle the conversion failure when parsing item Marcel Holtmann
2010-03-16 20:27     ` Denis Kenzior
2010-03-17  4:40       ` Marcel Holtmann
2010-03-16 20:29   ` Denis Kenzior
2010-03-17  3:33     ` Gu, Yang
2010-03-17  4:44       ` Marcel Holtmann [this message]
2010-03-16 20:28 ` [PATCH 1/4] Fix the structure of stk_subaddress Denis Kenzior

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=1268801083.2700.40.camel@localhost.localdomain \
    --to=marcel@holtmann.org \
    --cc=ofono@ofono.org \
    /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