From: Lars-Peter Clausen <email@example.com> To: Jonathan Cameron <firstname.lastname@example.org>, Dan Carpenter <email@example.com> Cc: Bhumika Goyal <firstname.lastname@example.org>, email@example.com, Michael.Hennerich@analog.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [PATCH 1/2] staging:iio:adc:added space around '-' Date: Sun, 24 Jan 2016 18:14:53 +0100 [thread overview] Message-ID: <56A5068D.firstname.lastname@example.org> (raw) In-Reply-To: <56A4FD7F.email@example.com> On 01/24/2016 05:36 PM, Jonathan Cameron wrote: > On 20/01/16 14:21, Dan Carpenter wrote: >> On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote: >>> On 01/15/2016 08:42 PM, Bhumika Goyal wrote: >>>> This patch adds apace around '-' operator.Found using checkpatch.pl >>>> >>>> Signed-off-by: Bhumika Goyal <firstname.lastname@example.org> >>>> --- >>>> drivers/staging/iio/adc/ad7280a.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c >>>> index f45ebed..0c73bce 100644 >>>> --- a/drivers/staging/iio/adc/ad7280a.c >>>> +++ b/drivers/staging/iio/adc/ad7280a.c >>>> @@ -744,14 +744,14 @@ out: >>>> } >>>> >>>> static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value, >>>> - in_voltage-voltage_thresh_low_value, >>>> + in_voltage - voltage_thresh_low_value, >>> >>> Hi, >>> >>> Thanks for patch. But when sending cleanup patches like this please make >>> sure that you a) understand what the code does and how your change affects >>> it and b) as a bare minimum of testing perform a compile test, if possible >>> also do functional testing. >>> >>> The patch as it is, is neither semantically nor syntactically correct. As an >>> exercise please make sure you understand why. >> >> Ugh! >> >> It took me a long time to figure out the bug in this patch... Why does >> that filename have a mix of dashes and underscores? Too late to fix it >> now... :/ >> > Very deliberately. The - is indicating it is a differential channel! > Literally A minus B. > > It's an awfully compact representation for maths ;) > This is obscured partly in this case as it's specifying an attribute > shared by a set of differential channels so it's the generalization > of > in_voltage0-voltage1_thresh_low_value > which does begin to slightly stretch the argument that it is nice and > clear ;( One thing we should maybe take a look at is making it explicit that this is a string so it does not get picked up by checkpatch.
next prev parent reply other threads:[~2016-01-24 17:51 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-15 19:42 [PATCH 0/2]Staging:iio:adc:add " Bhumika Goyal 2016-01-15 19:42 ` [PATCH 1/2] staging:iio:adc:added " Bhumika Goyal 2016-01-15 20:15 ` Lars-Peter Clausen 2016-01-20 14:21 ` Dan Carpenter 2016-01-24 16:36 ` Jonathan Cameron 2016-01-24 17:14 ` Lars-Peter Clausen [this message] 2016-01-30 14:12 ` Jonathan Cameron 2016-01-30 15:12 ` Dan Carpenter 2016-01-30 15:15 ` Lars-Peter Clausen 2016-01-30 17:02 ` Joe Perches 2016-01-15 19:42 ` [PATCH 2/2] staging:iio:adc:add " Bhumika Goyal
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=56A5068D.email@example.com \ --firstname.lastname@example.org \ --cc=Michael.Hennerich@analog.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 1/2] staging:iio:adc:added space around '\''-'\''' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).