linux-kernel - Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 2 Aug 2017 10:52:36 +0200 (CEST)
From:   Marcus Wolf <marcus.wolf@...f-entwicklungen.de>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Rishabh Hardas <rishabhhardas@...il.com>,
        gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        devel@...verdev.osuosl.org
Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from
 pi433_if.h and other dependencies arising from it.

Hi!
 
Ok. Didn't know that I have to check for such stuff.

So far i was just checking the code changes, not the style of the patch itself.

Will try to be more strict...

Is it mandatory, that I compile the code, or is it ok, if I do the review "just"
by reading? 
Reason for the question: If reading is ok, too, I can do a review of a simple
change,
even when I am abroad at a customer (my customers do not deal with linux, just 
comercial stuff). With compiling, I can only do them at home...

Cheers,

Marcus



> Dan Carpenter <dan.carpenter@...cle.com> hat am 2. August 2017 um 10:34
> geschrieben:
>
>
> On Wed, Aug 02, 2017 at 10:08:04AM +0200, Wolf Entwicklungen wrote:
> > Reviewed-by: Marcus Wolf <linux@...f-entwicklungen.de>
> >
> > 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
>
>
>