public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: Siarhei Siamashka <siarhei.siamashka@nokia.com>
Cc: BlueZ development <linux-bluetooth@vger.kernel.org>
Subject: Re: Bluez coding style again
Date: Fri, 23 Jan 2009 02:01:51 +0100	[thread overview]
Message-ID: <1232672511.10609.10.camel@californication> (raw)
In-Reply-To: <200901221759.37087.siarhei.siamashka@nokia.com>

Hi Siarhei,

> > > > > > --- a/sbc/sbc_tables.h
> > > > > > +++ b/sbc/sbc_tables.h
> > > > > > @@ -161,30 +161,30 @@ static const int32_t synmatrix8[16][8] = {
> > > > > >         ((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
> > > > > >  #define F(x) F_PROTO4(x)
> > > > > >  static const FIXED_T _sbc_proto_fixed4[40] = {
> > > > > > -        F(0.00000000E+00),  F(5.36548976E-04),
> > > > > > +       F(0.00000000E+00),  F(5.36548976E-04),
> > > > > >         -F(1.49188357E-03),  F(2.73370904E-03),
> > > > > > -        F(3.83720193E-03),  F(3.89205149E-03),
> > > > > > -        F(1.86581691E-03),  F(3.06012286E-03),
> > > > > > +       F(3.83720193E-03),  F(3.89205149E-03),
> > > > > > +       F(1.86581691E-03),  F(3.06012286E-03),
> > > > >
> > > > > With this latest commit, the vertical alignment of elements in tables
> > > > > gets messed up and it becomes harder to see any kind of symmetry in
> > > > > the tables. If the spaces are strictly forbidden, I would probably
> > > > > prefer replacing them with '+' character.
> > > >
> > > > that would be fine with me.
> 
> Well, I tried that with the last patch (constants tweaks for audio quality
> improvement), but 'checkpatch.pl' tool does not like it very much (it wants
> a space between '+' operator and 'F').
> 
> [...]
> 
> > > I'm just desperately trying to do everything in a right way, but still
> > > happen to violate one rule or another occasionally :)
> >
> > don't worry. Sometimes we have to bend the rules. The highest priority
> > is readability and make patch management simple for us ;)
> 
> I just returned some spaces as they improve vertical alignment and
> readability:
> 
> +	 F(2.73370904E-03 * C0),  F(5.36548976E-04 * C0),
> +	-F(1.49188357E-03 * C1),  F(0.00000000E+00 * C1),
> +	 F(3.83720193E-03 * C2),  F(1.09137620E-02 * C2),
> +	 F(3.89205149E-03 * C3),  F(3.06012286E-03 * C3),
> 
> I hope this should be ok.

fine with me and let Johan know so he doesn't fix it ;)

I am still behind applying your patches, but I will get to it. Just
mention that you really want it applied. Makes it easier for me to go
through the patch logs.
 
Regards

Marcel



      reply	other threads:[~2009-01-23  1:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-17 17:43 Bluez coding style again Siarhei Siamashka
2009-01-18 15:08 ` Marcel Holtmann
2009-01-20 16:48   ` Siarhei Siamashka
2009-01-21 11:21     ` Marcel Holtmann
2009-01-22 15:59       ` Siarhei Siamashka
2009-01-23  1:01         ` Marcel Holtmann [this message]

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=1232672511.10609.10.camel@californication \
    --to=marcel@holtmann.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=siarhei.siamashka@nokia.com \
    /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