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 11:34:07 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Wolf Entwicklungen <Marcus.Wolf@...f-entwicklungen.de>
Cc:     Rishabh Hardas <rishabhhardas@...il.com>,
        devel@...verdev.osuosl.org, gregkh@...uxfoundation.org,
        linux@...f-entwicklungen.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from
 pi433_if.h and other dependencies arising from it.

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ