Messages in this thread | | | Date | Tue, 13 Feb 2018 01:16:05 +0530 | From | Himanshu Jha <> | Subject | Re: [PATCH 2/4] staging: iio: accel: Remove unnecessary comments and add suitable suffix |
| |
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
| |