kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: me@tobin.cc (Tobin C. Harding)
To: kernelnewbies@lists.kernelnewbies.org
Subject: Patch Question
Date: Wed, 19 Apr 2017 10:11:32 +1000	[thread overview]
Message-ID: <20170419001132.GA15282@eros> (raw)
In-Reply-To: <CAO62RXcDbmKmxm+L72fcyKG6RRR3HvxW0W2nezQ4W=bazH-_QA@mail.gmail.com>

On Tue, Apr 18, 2017 at 05:07:08PM -0600, Perry Hooker wrote:
> Thanks for the advice, Tobin - I appreciate the reply.

Please don't top post http://daringfireball.net/2007/07/on_top

I'm not an endian expert so I will not comment on the technical
aspects of the path, I can however, comment on the thread and why you
may not be getting the response you desire.

> In this case, I've already followed your advice - I studied the
> reviewer's comments with a fine-toothed comb (some of his comments
> were flat-out incorrect)

Dan Carpenter is very good at what he dose. I would be hesitant to
ever call him or anyone as experienced 'flat-out incorrect'.

> , and traced the buffer in question back to
> its source. It appears to be holding host-endian data, and it's being
> cast to a little-endian type without an explicit conversion. The patch
> I submitted fixes this by using the kernel-defined byte-order macros.

His initial reply hints that this patch may need testing before it can
be applied - have you tested the patch on real hardware? If so, and it
is correct, re-submit the patch stating so.

> I've reached out to the reviewer both individually and via the mailing
> list, and haven't heard back.

>From the thread, and this is only my opinion, it seems Dan has put
more effort than is required of him already. No one is paid to answer
questions on LKML, they are not required to apply effort to your
problems, anything they do is a gift of their time and should be appreciated.

> It's possible that I'm missing something, but I don't see what.

In your comments you in no way display that you understand exactly
what the code is doing and why it should by changed. Your initial
patch does not have an appropriate changelog message, please read
Documentation/process/submitting-patches.rst (section 2 Describing
your changes).

A more subtle point - you may have more success if you do not put
demands onto people (eg can you explain this..) but rather write out
your understanding of the code explaining why your hold the views you
do. Others can then comment or this, agreeing or disagreeing as the
case may be. People like to help by giving their knowledge, no one
likes doing chores.

> At what point is it appropriate to re-submit the patch?

Once you have reworked the patch, taken into consideration the
reviewers comments, written a changelog describing the code as it is
and why it needs changing, explained the patch fully so that the previous
reviewer and future reviewers can understand that you understand what
is going on. Then it is appropriate to re-submit the patch ([PATCH v2]...).

Again, I am only new around here, these are my opinions based on what
I have seen and read. By no means should they be taken as gospel.

Remember, we are all here to make the kernel better. It's not
personal, it's about the kernel.

I hope this helps, best of luck.

Tobin.

  reply	other threads:[~2017-04-19  0:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-17 23:28 Patch Question Perry Hooker
2017-04-17 23:48 ` Mandeep Sandhu
2017-04-17 23:58 ` Tobin C. Harding
2017-04-18 23:07   ` Perry Hooker
2017-04-19  0:11     ` Tobin C. Harding [this message]
2017-04-19 15:54       ` Perry Hooker
2017-04-19 16:13         ` Greg KH
2017-04-19 16:46           ` Perry Hooker
2017-04-19 20:48             ` Lino Sanfilippo
2017-04-19 13:27 ` Lino Sanfilippo
2017-04-19 15:41   ` Perry Hooker

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=20170419001132.GA15282@eros \
    --to=me@tobin.cc \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /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;
as well as URLs for NNTP newsgroup(s).