Page 1 of 1

Repository push procedure

Posted: Thu Feb 27, 2014 8:52 pm
by domob
I've recently submitted two pull requests (for Namecoin-Qt and nmcontrol, I'm too lazy to look up links for now), and phelix ack'ed both of them. In theory, I have access to the repositories, but I'm hesitating to push changes myself to projects I consider maintained by others (I wouldn't hesitate to push NameID changes, of course).

I think we should define a general procedure for code review and commits. I suggest that either only the "main developer" of a particular software should merge patches (not me in the case of nmcontrol and Namecoin-Qt), or that we agree that everyone with write access can merge patches when they have been ok'ed by one or two others (like phelix' ack's).

What do you think?

Re: Repository push procedure

Posted: Fri Feb 28, 2014 6:06 pm
by phelix
domob wrote:I've recently submitted two pull requests (for Namecoin-Qt and nmcontrol, I'm too lazy to look up links for now), and phelix ack'ed both of them. In theory, I have access to the repositories, but I'm hesitating to push changes myself to projects I consider maintained by others (I wouldn't hesitate to push NameID changes, of course).

I think we should define a general procedure for code review and commits.
+1
I suggest that either only the "main developer" of a particular software should merge patches (not me in the case of nmcontrol and Namecoin-Qt), or that we agree that everyone with write access can merge patches when they have been ok'ed by one or two others (like phelix' ack's).

What do you think?
Not sure how to do it best. I could not yet find a description of Bitcoin's ACK procedure. I guess at least with some repos it would be best to have several people be able to merge.

Re: Repository push procedure

Posted: Mon Mar 03, 2014 6:57 am
by domob
phelix wrote:What do you think?
Not sure how to do it best. I could not yet find a description of Bitcoin's ACK procedure. I guess at least with some repos it would be best to have several people be able to merge.[/quote]
I can only tell you how GCC does it: There's a list of "maintainers" for each component, and those can push any changes they like for their own components. Also, there are some people who have "write-after-approval" rights. For those and if maintainers modify a different part of the code, they have to submit the patch to a mailing list, and if a maintainer of the respective component ok's the patch, they can commit.

For Mozilla, there was a two-tier system the last time I checked. Review by someone working on the specific part of the code that is modified, and "super-review" by one of a few persons who are familiar with the overall code-base. (But presumably the latter is only a rough check since they have a lot to do.)

We are a project smaller than either of those, but since we have (at least somewhat) to do with crypto and security, we should be careful, too. But I would be fine with an ok from one knowing the modified code - but I'm also fine if pull requests are solely merged by the main devs of the code in question.

Re: Repository push procedure

Posted: Mon Mar 03, 2014 11:15 am
by khal
Github provides us an easy way to manage pull requests from people not having the right to push/merge to the repo. As a consequence, we do not need a lot of people to have a push right, while still allowing participation.

I'm fine with an ok from another person knowing the code, for non controversial commits & for commits that don't change how namecoin works.
People who have push/merge access can use it, that's what it is done for (if they use their right badly, they'll be whipped :p)

Re: Repository push procedure

Posted: Tue Mar 04, 2014 8:19 am
by phelix
ok. So anybody with push access can and should use it. Critical and large changes need to get more ACKs.

At least one person needs to review/test and ACK a pull.