Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

On Mon, Jun 19, 2017 at 10:41:54AM -0300, Antonio Terceiro wrote:
> I would imagine that any PR workflow would allow anyone to add a comment
> saying they have reviewed the changes and they look good, withing the
> same "place" where the maintainer is already looking at.

Indeed, but it would be useful to be able to mark changes are "package
uploader approved for upload".  Possibly this could be done with PR tags
that can only be set by the package maintainer, but I think my DEP's
implementation is simpler.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Christian Seiler]

On 06/19/2017 05:37 PM, Sean Whitton wrote:
> On Mon, Jun 19, 2017 at 08:56:14AM -0400, Jeremy Bicha wrote:
>> On Mon, Jun 19, 2017 at 5:18 AM, Sean Whitton  
>> wrote:
>>> Someone might contribute a fix in the form of a PR, and an uploader of
>>> the package might review that fix and determine that it should be
>>> merged.  They then look at the master branch and decide that it should
>>> not go into the next upload, for whatever reason.  So they can merge the
>>> PR to next/sid.
>>
>> Respectfully, why?
> 
> There are various situations in which this could come up.
> 
> A package might have multiple uploaders, with one or two of them working
> on a big, potentially disruptive upload.  A third uploader, not involved
> in the current work, might want to review and merge uploadable changes,
> without interfering with the work going on in the master branch.  They
> can put them on the next/foo branch.

But that doesn't need a separate area to push this with special
restrictions for who can push there: this can just be done into
a separate branch in the repository by the people who already
have access to said repository. And from a maintainer point of
view: it's far simpler for me to simply do a push to the actual
packaging repository of a new branch than to push to a new
remote specifically created for this.

If you want to standardize this a bit, why not update DEP14 then
to include recommendations for how these kinds of branches should
be named? Even though I do think this is going to be a rather
rare situation, I can see the merit of having a recommended
convention for this. But that would fit far better into DEP14
IMHO.

But I still fail to see any advantage of the technical solution
you suggested in the start of the thread, with all the permission
checks and so on.

Regards,
Christian

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Scott Kitterman]

On Monday, June 19, 2017 04:37:12 PM Sean Whitton wrote:
> Hello Jeremy,
> 
> On Mon, Jun 19, 2017 at 08:56:14AM -0400, Jeremy Bicha wrote:
> > On Mon, Jun 19, 2017 at 5:18 AM, Sean Whitton  
wrote:
> > > Someone might contribute a fix in the form of a PR, and an uploader of
> > > the package might review that fix and determine that it should be
> > > merged.  They then look at the master branch and decide that it should
> > > not go into the next upload, for whatever reason.  So they can merge the
> > > PR to next/sid.
> > 
> > Respectfully, why?
> 
> There are various situations in which this could come up.
> 
> A package might have multiple uploaders, with one or two of them working
> on a big, potentially disruptive upload.  A third uploader, not involved
> in the current work, might want to review and merge uploadable changes,
> without interfering with the work going on in the master branch.  They
> can put them on the next/foo branch.
> 
> Alternatively, someone might be burnt out by their work on the master
> branch.  They want to put aside the next upload for a while, and review
> some other fixes that have been submitted.  Having a next/foo branch to
> which they can commit these reviewed changes permits them to properly
> step away from the master branch, relieving them of the need to think
> hard about which changes can be safely combined with the upcoming
> upload.
> 
> > And why does that unusual workflow need a DEP?
> 
> To reserve the next/foo namespace for this purpose.

I'm sorry, but this is sounding more and more like a solution in search of a 
problem.  Are there actual problems that actual maintainers are having right 
now that this solves?

Scott K

signature.asc
Description: This is a digitally signed message part.

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Hello Jeremy,

On Mon, Jun 19, 2017 at 08:56:14AM -0400, Jeremy Bicha wrote:
> On Mon, Jun 19, 2017 at 5:18 AM, Sean Whitton  
> wrote:
> > Someone might contribute a fix in the form of a PR, and an uploader of
> > the package might review that fix and determine that it should be
> > merged.  They then look at the master branch and decide that it should
> > not go into the next upload, for whatever reason.  So they can merge the
> > PR to next/sid.
> 
> Respectfully, why?

There are various situations in which this could come up.

A package might have multiple uploaders, with one or two of them working
on a big, potentially disruptive upload.  A third uploader, not involved
in the current work, might want to review and merge uploadable changes,
without interfering with the work going on in the master branch.  They
can put them on the next/foo branch.

Alternatively, someone might be burnt out by their work on the master
branch.  They want to put aside the next upload for a while, and review
some other fixes that have been submitted.  Having a next/foo branch to
which they can commit these reviewed changes permits them to properly
step away from the master branch, relieving them of the need to think
hard about which changes can be safely combined with the upcoming
upload.

> And why does that unusual workflow need a DEP?

To reserve the next/foo namespace for this purpose.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Antonio Terceiro]

On Mon, Jun 19, 2017 at 10:18:52AM +0100, Sean Whitton wrote:
> Hello Christian,
> 
> On Sun, Jun 11, 2017 at 08:16:43PM +0100, Sean Whitton wrote:
> > Christian Seiler  writes:
> > 
> > > Your goal in wanting to stop people from having to deal with
> > > patch files manually is laudable, but I see the following way
> > > forward to achieve that goal:
> > >
> > >  - Pull requests.
> > >
> > >  - Make it easier to create personal copies of remote (!)
> > >repositories in one's own space. (Currently it's still a bit
> > >cumbersome.)
> > 
> > This would cover most of the use cases I had in mind.  Thanks for
> > bringing it up.
> 
> Since writing this I've thought of another usecase, where next/foo
> branches complement pull requests.
> 
> Someone might contribute a fix in the form of a PR, and an uploader of
> the package might review that fix and determine that it should be
> merged.  They then look at the master branch and decide that it should
> not go into the next upload, for whatever reason.  So they can merge the
> PR to next/sid.
> 
> This is useful because it avoids accidentally reviewing the patch twice.

I would imagine that any PR workflow would allow anyone to add a comment
saying they have reviewed the changes and they look good, withing the
same "place" where the maintainer is already looking at.


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Jeremy Bicha]

On Mon, Jun 19, 2017 at 5:18 AM, Sean Whitton  wrote:
> Someone might contribute a fix in the form of a PR, and an uploader of
> the package might review that fix and determine that it should be
> merged.  They then look at the master branch and decide that it should
> not go into the next upload, for whatever reason.  So they can merge the
> PR to next/sid.

Respectfully, why? And why does that unusual workflow need a DEP?

Thanks,
Jeremy Bicha

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Hello Christian,

On Sun, Jun 11, 2017 at 08:16:43PM +0100, Sean Whitton wrote:
> Christian Seiler  writes:
> 
> > Your goal in wanting to stop people from having to deal with
> > patch files manually is laudable, but I see the following way
> > forward to achieve that goal:
> >
> >  - Pull requests.
> >
> >  - Make it easier to create personal copies of remote (!)
> >repositories in one's own space. (Currently it's still a bit
> >cumbersome.)
> 
> This would cover most of the use cases I had in mind.  Thanks for
> bringing it up.

Since writing this I've thought of another usecase, where next/foo
branches complement pull requests.

Someone might contribute a fix in the form of a PR, and an uploader of
the package might review that fix and determine that it should be
merged.  They then look at the master branch and decide that it should
not go into the next upload, for whatever reason.  So they can merge the
PR to next/sid.

This is useful because it avoids accidentally reviewing the patch twice.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Christian Seiler  writes:

> Your goal in wanting to stop people from having to deal with
> patch files manually is laudable, but I see the following way
> forward to achieve that goal:
>
>  - Pull requests.
>
>  - Make it easier to create personal copies of remote (!)
>repositories in one's own space. (Currently it's still a bit
>cumbersome.)

This would cover most of the use cases I had in mind.  Thanks for
bringing it up.  Let's see what Ian thinks.

-- 
Sean Whitton

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Christian Seiler  writes:

> Your goal in wanting to stop people from having to deal with
> patch files manually is laudable, but I see the following way
> forward to achieve that goal:
>
>  - Pull requests.
>
>  - Make it easier to create personal copies of remote (!)
>repositories in one's own space. (Currently it's still a bit
>cumbersome.)

This would 

-- 
Sean Whitton

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Henrique de Moraes Holschuh]

On Sun, 11 Jun 2017, Sean Whitton wrote:
> Henrique de Moraes Holschuh  writes:
> 
> > Not just that.  If it is for NMUs, one also has to ensure it matches
> > what got uploaded (regardless of method: NMU patch, PR, branch...).
> 
> I'm not sure what you're getting at here -- this DEP is for changes that
> *aren't* to be uploaded as part of an NMU.

The (thread?) naming is unfortunate, then.  Anyway, if it does not
involve NMUs, my comment does not apply.

-- 
  Henrique Holschuh

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Christian Seiler]

On 06/11/2017 07:44 PM, Sean Whitton wrote:
> Christian Seiler  writes:
> 
>> To me this looks like a very complicated technical solution
>> to something that I've never encountered as a problem myself.
> 
> Could you explain which parts of the proposal you find to be "very
> complicated"?  Possibly I've made them seem much more complicated than
> they actually are.

Well, the "very complicated" was mostly geared at the required
infrastructure behind it. Sure, you can reuse some dgit server
code, but in the end someone needs to maintain this.

> If a package does not have a repo on alioth, the only way for me to
> contribute a fix is to NMU, which always creates work for the
> maintainer, or file a bug report with patches.
> 
> With this DEP, I can push a next/foo branch, and file a bug pointing to
> it.  This means neither the maintainer nor contributor need mess around
> with patch files.

You can already create user repositories on Alioth where you can
push stuff. And I suspect that the successor of Alioth will
provide something similar.

My perspective is the following:

 - Most contributions to packages I'm (co-)maintaining have come
   from non-DDs.

 - I don't care whether a contribution comes from a DD or not. I
   always review it before committing it to the package.

So from this perspective you're trying to solve a non-problem,
and throwing a lot of infrastructure at it to do so.

Your goal in wanting to stop people from having to deal with
patch files manually is laudable, but I see the following way
forward to achieve that goal:

 - Pull requests.

 - Make it easier to create personal copies of remote (!)
   repositories in one's own space. (Currently it's still a bit
   cumbersome.)

Whether changes were done by a DD or not is not relevant for
anything that's not an archive upload, IMHO. In the archive it
_is_ relevant as packages get accepted automatically with a
proper signature. But any time there's a person doing manual
review of something (and every maintainer should!), I really
don't see any advantage whatsoever.

Regards,
Christian

[1] You mentioned that DMs with upload permissions for specific
packages would also be allowed, but I think you could ignore
that completely: a DM will either have permissions to upload
a specific package, in which case they're already a (co-)
maintainer of said package and should have access rights to
the packaging repository, or they don't, in which case they
won't be able to push there.

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Christian Seiler  writes:

> To me this looks like a very complicated technical solution
> to something that I've never encountered as a problem myself.

Could you explain which parts of the proposal you find to be "very
complicated"?  Possibly I've made them seem much more complicated than
they actually are.

> Again, sorry that I'm so negative here, and of course I have my own
> biases, but maybe you could provide an example work- flow where your
> proposal actually helps the maintainer and/ or the contributor?

Don't apologise for providing feedback :)  I'm grateful for it.

After reading comments in this thread, I think that the main use of
next/foo branches is likely to be for packages which lack repos on
alioth.  Otherwise, as you say, the contributor could use a PR.

If a package does not have a repo on alioth, the only way for me to
contribute a fix is to NMU, which always creates work for the
maintainer, or file a bug report with patches.

With this DEP, I can push a next/foo branch, and file a bug pointing to
it.  This means neither the maintainer nor contributor need mess around
with patch files.

-- 
Sean Whitton

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Ansgar Burchardt  writes:

> What about contributions to non-packaged parts of Debian?

This DEP isn't about those -- are you saying that it ought to be
extended?

> I also don't like having more systems only a subset of contributors
> can use.

I share this concern, but it is easier for a DD to review a change and
submit it to a next/foo branch than it is for a DD to review an NMU and
try to figure out whether it makes sense for an upload to happen right
now.  So it's more accessible to those without upload rights than NMUs
are at present.

> Having a place where every contributor can publish merge requests is
> nicer (one can still have a tool to make it easier to check commit
> signatures using the Debian keyrings if one so desires).

Not all packages have a repo on alioth, so even when alioth runs pagure
and gains PRs, there won't always be a place to submit changes.  With
this DEP, there is a place to submit a next/foo branch for any package
in the archive.

-- 
Sean Whitton

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Henrique de Moraes Holschuh  writes:

> Not just that.  If it is for NMUs, one also has to ensure it matches
> what got uploaded (regardless of method: NMU patch, PR, branch...).

I'm not sure what you're getting at here -- this DEP is for changes that
*aren't* to be uploaded as part of an NMU.

-- 
Sean Whitton

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Henrique de Moraes Holschuh]

On Fri, 09 Jun 2017, Tollef Fog Heen wrote:
> > Per the DEP:
> > 
> > > it is very useful for a maintainer to know that a change has been
> > > approved by someone who has been trusted by the project with the
> > > technical ability to NMU the package
> > 
> > This would be much more cumbersome to achieve with PRs.
> 
> I'm not sure why this is very useful.  It can, in some cases, be a
> useful data point, but in general, as the maintainer, I'll want to
> review the patch in the same way no matter whether it came from somebody
> with a key in the keyring or not.

Not just that.  If it is for NMUs, one also has to ensure it matches
what got uploaded (regardless of method: NMU patch, PR, branch...).

-- 
  Henrique Holschuh

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Tollef Fog Heen]

]] Sean Whitton 

> Per the DEP:
> 
> > it is very useful for a maintainer to know that a change has been
> > approved by someone who has been trusted by the project with the
> > technical ability to NMU the package
> 
> This would be much more cumbersome to achieve with PRs.

I'm not sure why this is very useful.  It can, in some cases, be a
useful data point, but in general, as the maintainer, I'll want to
review the patch in the same way no matter whether it came from somebody
with a key in the keyring or not.

-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Christian Seiler]

Hi Sean,

Am 2017-06-07 22:56, schrieb Sean Whitton:

I am hereby reserving DEP number 15 for my draft DEP, "Reserved
namespaces for DD-approved non-maintainer changes".

I'd like to suggest discussing this DEP on d-devel (which is the
Reply-to for this e-mail).  The canonical DEP text is at
.


I really hate to be negative here, but I just don't see the
point of it.

If I look at how I maintain packages, if someone sends me a
patch (or later once implemented a pull request), I will
always carefully look at that patch - regardless of who it
is from. Not because I necessarily mistrust people, but just
so I also am up to speed about the current state of the
package. I really don't see how having an area to pull from
where only DDs and selected DMs can push to would help here.
(And if contributions from a specific person come often
enough, I'd suggest co-maintainership anyway.)

To me this looks like a very complicated technical solution
to something that I've never encountered as a problem myself.

Again, sorry that I'm so negative here, and of course I have
my own biases, but maybe you could provide an example work-
flow where your proposal actually helps the maintainer and/
or the contributor?

Regards,
Christian

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Ansgar Burchardt]

Sean Whitton writes:
> I am hereby reserving DEP number 15 for my draft DEP, "Reserved
> namespaces for DD-approved non-maintainer changes".
>
> I'd like to suggest discussing this DEP on d-devel (which is the
> Reply-to for this e-mail).  The canonical DEP text is at
> .

What about contributions to non-packaged parts of Debian?

I also don't like having more systems only a subset of contributors can
use.  Having a place where every contributor can publish merge requests
is nicer (one can still have a tool to make it easier to check commit
signatures using the Debian keyrings if one so desires).

Ansgar

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

Hello Jeremy,

On Wed, Jun 07, 2017 at 05:06:10PM -0400, Jeremy Bicha wrote:
> On Wed, Jun 7, 2017 at 4:56 PM, Sean Whitton  wrote:
> > I am hereby reserving DEP number 15 for my draft DEP, "Reserved
> > namespaces for DD-approved non-maintainer changes".
> 
> I think this proposal somewhat contradicts DEP-14.

I forgot to include in the draft an explanation for why this is separate
to DEP-14.  DEP-14's reserved namespaces are meant to be present in
every clone of the repository.  By contrast, DEP-15 reserves a namespace
that should not be present in every clone.

That being said, I don't see why it /contradicts/ DEP-14.  Perhaps you
could explain.

> It also feels to me like this proposal is another way of pushing dgit?

It doesn't require dgit.  I tried to state this as clearly as I could in
the text of the DEP.  dgit-repos is the suggested implementation, but
from the point of view of a DD/DM, it's just a git remote that requires
PGP-signed pushes, which is part of upstream git.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Sean Whitton]

On Thu, Jun 08, 2017 at 05:41:22AM +0800, Boyuan Yang wrote:
> Just wondering why we need to control the branch name of proposed topic
> branch, or even use a single repo to receive contributions.
> 
> My thought is that all we need is the "Pull Request / Merge Request" feature 
> on Alioth, where random contributors (who might have absolutely no permission 
> anywhere) can create forks (forked repositories) freely and request for 
> review 
> and eventual merge from the forked repo into main repo if necessary. People 
> are much familiar with such workflow similar to GitHub / GitLab's fork-and-
> merge behaviour.

We also want merge requests!  This DEP's implementation should live
alongside those.

Per the DEP:

> it is very useful for a maintainer to know that a change has been
> approved by someone who has been trusted by the project with the
> technical ability to NMU the package

This would be much more cumbersome to achieve with PRs.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Ian Jackson]

Sean Whitton writes ("DEP 15: Reserved namespace for DD-approved non-maintainer 
changes"):
> I am hereby reserving DEP number 15 for my draft DEP, "Reserved
> namespaces for DD-approved non-maintainer changes".
> 
> I'd like to suggest discussing this DEP on d-devel (which is the
> Reply-to for this e-mail).  The canonical DEP text is at
> .
> 
> The drivers are myself and Ian Jackson, who came up with the idea, but I
> said I'd write up the formal proposal.

Thanks, Sean.

For others: this was prompted by a conversation Sean and I had in a
pub on Monday.

Before we get into a detailed discussion here on -devel I think it
would be worth Sean and me getting the DEP draft into some kind of
reasonable shape that we both agree on.

So I won't reply in detail to the comments.

Ian.

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Boyuan Yang]

在 2017年6月7日星期三 +08 下午9:56:39，Sean Whitton 写道：
> Dear all,
> 
> I am hereby reserving DEP number 15 for my draft DEP, "Reserved
> namespaces for DD-approved non-maintainer changes".
> 
> I'd like to suggest discussing this DEP on d-devel (which is the
> Reply-to for this e-mail).  The canonical DEP text is at
> .
> 
> The drivers are myself and Ian Jackson, who came up with the idea, but I
> said I'd write up the formal proposal.

Just wondering why we need to control the branch name of proposed topic 
branch, or even use a single repo to receive contributions.

My thought is that all we need is the "Pull Request / Merge Request" feature 
on Alioth, where random contributors (who might have absolutely no permission 
anywhere) can create forks (forked repositories) freely and request for review 
and eventual merge from the forked repo into main repo if necessary. People 
are much familiar with such workflow similar to GitHub / GitLab's fork-and-
merge behaviour.

That would also be much easier than setting up custom access control rules 
based on permission inside a single repository.

--
Boyuan Yang

signature.asc
Description: This is a digitally signed message part.

Re: DEP 15: Reserved namespace for DD-approved non-maintainer changes

[Jeremy Bicha]

On Wed, Jun 7, 2017 at 4:56 PM, Sean Whitton  wrote:
> I am hereby reserving DEP number 15 for my draft DEP, "Reserved
> namespaces for DD-approved non-maintainer changes".

I think this proposal somewhat contradicts DEP-14.

It also feels to me like this proposal is another way of pushing dgit?

Thanks,
Jeremy Bicha