From: Dan Carpenter <dan.carpenter@oracle.com>
To: Wolf Entwicklungen <Marcus.Wolf@wolf-entwicklungen.de>
Cc: Rishabh Hardas <rishabhhardas@gmail.com>,
devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
linux@wolf-entwicklungen.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] staging/pi433:Removed Coding style issues from pi433_if.h and other dependencies arising from it.
Date: Wed, 2 Aug 2017 11:34:07 +0300 [thread overview]
Message-ID: <20170802083406.6d6mtli7pm2w7zrr@mwanda> (raw)
In-Reply-To: <88d4ff61d322e563ffd52f7375227f8d-EhVcX1pHQwdXWkQFBhENSgEKLlwACzJXX19HAVhEWENbS1kLMF52CEtUX1pBSEwcXlJRL1lQWAlZWXcDXVE=-webmailer1@server03.webmailer.webmailer.hosteurope.de>
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
next prev parent reply other threads:[~2017-08-02 8:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-01 19:31 Rishabh Hardas
2017-08-01 19:31 ` [PATCH 2/5] staging/pi433/pi433_if.c:Removed " Rishabh Hardas
2017-08-02 8:09 ` Wolf Entwicklungen
2017-08-02 8:26 ` Dan Carpenter
2017-08-01 19:31 ` [PATCH 3/5] staging/pi433/rf69.h:Removed " Rishabh Hardas
2017-08-02 8:10 ` Wolf Entwicklungen
2017-08-01 19:31 ` [PATCH 4/5] staging/pi433/rf69.c:Removed " Rishabh Hardas
2017-08-02 8:13 ` Wolf Entwicklungen
2017-08-01 19:31 ` [PATCH 5/5] staging/pi433/rf69_enum.h:Removed " Rishabh Hardas
2017-08-02 8:14 ` Wolf Entwicklungen
2017-08-02 8:08 ` [PATCH 1/5] staging/pi433:Removed " Wolf Entwicklungen
2017-08-02 8:34 ` Dan Carpenter [this message]
2017-08-02 8:52 ` Marcus Wolf
2017-08-02 8:59 ` Dan Carpenter
2017-08-02 9:15 ` Wolf Entwicklungen
2017-08-02 9:33 ` Dan Carpenter
[not found] ` <CAOo1wfVa6HoRLK9nXmnGotQ9n_hVmB01Osm1XjgJf3J_vqrY_g@mail.gmail.com>
2017-08-02 10:51 ` Dan Carpenter
2017-08-02 8:19 ` Dan Carpenter
2017-08-02 8:23 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170802083406.6d6mtli7pm2w7zrr@mwanda \
--to=dan.carpenter@oracle.com \
--cc=Marcus.Wolf@wolf-entwicklungen.de \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@wolf-entwicklungen.de \
--cc=rishabhhardas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®