Re: [PATCH 2/4] staging: iio: accel: Remove unnecessary comments and add suitable suffix

From: Himanshu Jha
Date: Mon Feb 12 2018 - 14:46:28 EST


On Mon, Feb 12, 2018 at 05:57:31PM +0300, Dan Carpenter wrote:
> On Mon, Feb 12, 2018 at 08:05:22PM +0530, Himanshu Jha wrote:
> > But these should be done when we have *more* instances.
> >
> > For eg:
> > I added a tab space in function static int adis16201_read_raw() argument
> > to match open parentheses in this patch. But I also added tabs while
> > removing and adding suitable suffix to the macros. So, should it also be
> > done in a separate patch ?
>
> If you're changing a line of code and you fix a white space issue on
> that same line, then that's fine. If it's just in the same function,
> then do it in a separate patch. In other words, adding tabs when you're
> moving around macros is fine, but adding it to the arguments is
> unrelated.

I will try and divide my patches next time :)

> This patch was honestly pretty tricky to review.

I am sorry for that. Might be easy for IIO reviewers ;)

> Jonathan assumes reviewers have the datasheet in front of them and I
> assume that that they don't. He's probably right... But especially
> comments like this:
>
> *val2 = 220000; /* 1.22 mV */

They are pretty obvious as you can see from the return statements
just below that which is :

return IIO_VAL_INT_PLUS_MICRO;

These comments are obvious because we know 'val1' will be responsible
for Integer part(1.0) and 'val2' for the Micro part(220000 * 10^-6 = 0.22).
Isn't it ?

Although I am new to IIO please correct if I'm wrong.

--
Thanks
Himanshu Jha