Date: Wed, 2 Aug 2017 10:52:36 +0200 (CEST)
From: Marcus Wolf <marcus.wolf@wolf-entwicklungen.de>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Rishabh Hardas <rishabhhardas@gmail.com>, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org
Message-ID: <765971804.35469.1501663956377@ox.hosteurope.de>
In-Reply-To: <20170802083406.6d6mtli7pm2w7zrr@mwanda>
References: <1501615919-31875-1-git-send-email-rishabhhardas@gmail.com>
 <88d4ff61d322e563ffd52f7375227f8d-EhVcX1pHQwdXWkQFBhENSgEKLlwACzJXX19HAVhEWENbS1kLMF52CEtUX1pBSEwcXlJRL1lQWAlZWXcDXVE=-webmailer1@server03.webmailer.webmailer.hosteurope.de>
 <20170802083406.6d6mtli7pm2w7zrr@mwanda>
Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from
 pi433_if.h and other dependencies arising from it.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Importance: Medium
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1793
Lines: 63

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@oracle.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@wolf-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
>
>
>