[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: patch to fix short-option for certfile (-F) in nbd-client



Hi Eric,

On Tue, Jul 25, 2023 at 02:52:13PM -0500, Eric Blake wrote:
> On Tue, Jul 25, 2023 at 10:07:34AM -0400, Matt Panaro wrote:
> > I like the second option that avoids the #else branches, even at the cost
> > of losing the alphabetical ordering.  this is the first time I've submitted
> > a code-change through email rather than e.g. GitHub, so I'm a bit uncertain
> > about how to proceed. should I reply with an updated diff? start a new
> > thread?
> 
> I generally start a new thread if posting a v2 patch, otherwise reply
> to the existing thread when adding more ideas on where the development
> might go.  Also, technical lists tend to favor inline replies (as I'm
> doing here), rather than top-posting.
> 
> Although the project is hosted on gitlab, Wouter would be the one to
> say whether he's comfortable with gitlab's pull request model,

I'm not really opposed to pull requests, but given that there are more
people reading this mailinglist than there are who review pull requests,
for larger patches sending them here is preferred.

IOW, trivial, obviously-correct, patches like the one at the head of
this thread are fine through the github PR workflow, but for larger
things, send them to this list instead.

> or whether he still prefers patches by email (I personally find email
> easier to work with, and am not sure I have the rights to approve a
> gitlab pull request although I do have the rights to push your patch
> on your behalf if it came through email and is ready to go).

I believe you do have the relevant rights to do that, although only you
would be able to check :-)

> There are ways for a maintainer to use an email-based flow even when
> contributors prefer a gitlab-based UI flow, but not all projects are
> set up that way.

Github has some form of "subscribe" button somewhere in the UI which
makes all PRs be sent through email, too. Although then you don't get
the patch and you'll have to manually pull the branch from the PR. A bit
more involved. I don't see it as a replacement, really.

> > also, who would I ask for/how would I get the "signed off by"?
> 
> Git has the notion of a 'Signed-off-by:' tag that can be added to a
> commit with project-dependent semantics; 'git commit --amend -s' is
> one easy way to add it to an existing patch.  The Linux kernel has
> well-defined semantics for what it means[1] (the tl;dr: "adding S-o-b
> is a legal statement that my patch won't break the kernel's license"),
> but other projects are laxer and some will accept submissions without
> S-o-b.  Given how the nbd project is at the intersection of kernel and
> userspace code, I personally use S-o-b for patches here, but Wouter as
> the project owner has final say on whether it is a hard requirement
> for NBD (you can find older commits in nbd.git that lacked one, and
> 'git grep -i "certificate of origin"' has no hits yet).

I tend to agree that it's probably a good idea to have them for NBD, but
I keep forgetting to add them myself, and so it seems... improper? to
then require it from others ;-)

[...]
> > one more thing: there's a slightly larger additional change/feature I'd
> > like to implement: is the correct thing to do start a codeless discussion
> > on the mailing list? or just send a larger diff (with appropriate
> > references to commit-hashes)?
> 
> Either is fine for me - getting consensus on the general idea of the
> feature before spending time writing the patches may be the smoothest
> path forward, but sometimes patches speak for themselves.

Agreed. If you're not sure whether your patch would even be accepted and
want to discuss the general idea first, that's certainly welcome. If you
just send a patch and it's not quite what I would like to see, however,
I'm happy to work with you to massage it into something that will be
more acceptable.

-- 
     w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}

I will have a Tin-Actinium-Potassium mixture, thanks.


Reply to: