netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: "Štěpán Němec" <snemec@redhat.com>
Cc: Phil Sutter <phil@nwl.cc>, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nft 1/3] tests: shell: README: copy edit
Date: Wed, 27 Oct 2021 11:06:00 +0200	[thread overview]
Message-ID: <YXkWeFPM3ixQ2cTb@salvia> (raw)
In-Reply-To: <20211021130323+0200.342006-snemec@redhat.com>

On Thu, Oct 21, 2021 at 01:03:23PM +0200, Štěpán Němec wrote:
> On Thu, 21 Oct 2021 12:26:44 +0200
> Phil Sutter wrote:
> 
> > Sorry for not checking the guideline but quoting advice I once received
> > from the top of my head instead. Maybe I was incorrect and in obvious
> > cases the description is optional. The relevant text in [2] at least
> > doesn't explicitly state it is, while being pretty verbose about it
> > regarding cover letters.
> 
> Does this mean that you retract your requirement? If not, could you
> please make sure that you and the other maintainers are on the same page
> about this, and document the requirement?
> 
> Regarding this patch series (if it is to be resent, in part or as a
> whole): we've discussed the first patch so far; the other two patches
> have no description, either. Do they need one? If so, could you provide
> some suggestions? I can't think of anything sensible that isn't already
> said in the subject, Fixes:, or the modified README text itself.

I also prefer if there is oneline description in the patch. My
suggestions:

* patch 1/3, not clear to me what "copy edit" means either.

* patch 2/3, what is the intention there? a path to the nft executable
  is most generic way to refer how you use $NFT, right?

* patch 3/3, I'd add a terse sentence so I do not need to scroll down
  and read the update to README to understand what this patch updates.
  I'd suggest: "Test files are located with find, so they can be placed
  in any location."

Regarding reference to 4d26b6dd3c4c, not sure it is worth to add this
to the README file. The test infrastructure is only used for internal
development use, this is included in tarballs but distributors do not
package this.

So please address Phil's feedback.

Thanks.

  reply	other threads:[~2021-10-27  9:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-20 12:45 [PATCH nft 1/3] tests: shell: README: copy edit Štěpán Němec
2021-10-20 12:45 ` [PATCH nft 2/3] tests: shell: README: $NFT does not have to be a path to a binary Štěpán Němec
2021-10-20 12:45 ` [PATCH nft 3/3] tests: shell: README: clarify test file name convention Štěpán Němec
2021-10-20 15:04 ` [PATCH nft 1/3] tests: shell: README: copy edit Phil Sutter
2021-10-21  8:30   ` Štěpán Němec
2021-10-21 10:26     ` Phil Sutter
2021-10-21 11:03       ` Štěpán Němec
2021-10-27  9:06         ` Pablo Neira Ayuso [this message]
2021-10-27  9:51           ` Štěpán Němec
2021-10-27 10:13             ` Pablo Neira Ayuso
2021-10-27 11:04               ` Štěpán Němec
2021-10-27 19:07                 ` Pablo Neira Ayuso
2021-11-05 11:39                   ` [PATCH nft v2 1/4] " Štěpán Němec
2021-11-05 11:39                     ` [PATCH nft v2 2/4] tests: shell: README: $NFT does not have to be a path to a binary Štěpán Němec
2021-11-05 11:39                     ` [PATCH nft v2 3/4] tests: shell: README: clarify test file name convention Štěpán Němec
2021-11-05 11:39                     ` [PATCH nft v2 4/4] tests: shell: $NFT needs to be invoked unquoted Štěpán Němec
2021-11-05 13:22                     ` [PATCH nft v2 1/4] tests: shell: README: copy edit Phil Sutter

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=YXkWeFPM3ixQ2cTb@salvia \
    --to=pablo@netfilter.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    --cc=snemec@redhat.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;
as well as URLs for NNTP newsgroup(s).