Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.
From: Dan Carpenter
Date: Wed Aug 02 2017 - 04:34:24 EST
On Wed, Aug 02, 2017 at 10:08:04AM +0200, Wolf Entwicklungen wrote:
> Reviewed-by: Marcus Wolf <linux@xxxxxxxxxxxxxxxxxxxxx>
>
> Just reviewed, not tested.
> As far as I can see, there is no technical issue with this patch.
You need to be a bit more strict in your reviews... There were a few
obvious problems in this patchset. These are show stoppers:
1) Breaks git bisect
2) Doing multiple things in the same patch
3) No changelog
>
> I prefer the names of the enumerations in camel case, because then they are a bit shorter.
> If camel case is unwanted, for sure we need that change.
Camel case are unwanted.
>
> Please mind the allignment. For enhanced readability of structs, I always try to start the type
> in the same column, the variable name in the same column and - if nneded - the comments in the
> same column - so you see all members of the struct optically in a kind of table.
Rishabh is going to have to redo the patchset anyway so don't feel bad
about asking for changes. Put these review comments next to the change
you are complaining about.
No top posting.
regards,
dan carpenter