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@xxxxxxxxxx> 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@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 > > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
