Thanks for the clarification Stian. :) On Thu, May 12, 2016 at 7:04 AM Stian Soiland-Reyes <[email protected]> wrote:
> Anyone with committer rights, e.g. who is listed on our /about page, should > have git write access. > > Some projects practice a review-than-commit process, where a committer > deliberately makes a pull request so it can be reviewed, it is then either > merged by whoever reviews it, or If a plenary discussion shows everything > is good, the committer merges his own commit. > > I hope Taverna don't need such a process, as we are a bit small! :-) So I > think we practice "just do it" instead, commit what you have, and then > let's see what breaks (..) ;) > > But it would still be appropriate for a big change to be reviewed, and a > branch (perhaps on the asf repo so others can change it) followed by a pull > request to form the discussion thread for the proposed change. > > Obviously for pull requests from non-committers we need to review them. I > think we've just practised "first one wins", with GSOC contributions > generally reviewed by the corresponding author. But I'm OK with anyone else > reviewing and accepting my GSOC student's pull requests if they get there > first! > > On 12 May 2016 2:43 p.m., "Gale Naylor" <[email protected]> > wrote: > > > A couple of questions regarding merging pull requests: Who is able to > > merge? Is the pull request typically reviewed by someone other than the > > author? I'll try to find a place to document this. Any suggestions? > > > > On Thu, May 12, 2016, 4:09 AM Stian Soiland-Reyes <[email protected]> > > wrote: > > > > > If you merge it normally or with --no-ff, then you usually don't need > > > the magic "This closes #1" string, as GitHub detects it's the same > > > commit ids. > > > > > > > > > If you do a squash merge or similar, then you would need "This closes > > #1". > > > > > > I think if we get code from outside, then it's good if we use merge > > > --no-ff (this forces a merge commit), which commit message can have > > > the "This closes #1" to be relate it to the pull request. > > > > > > That way we can also see in the git log which of the committers > > > reviewed the patch and also track the link to the pull request. > > > > > > (You can always see who pushed it in the commit mailing list, e.g. > > > > > > > > > http://mail-archives.apache.org/mod_mbox/taverna-commits/201605.mbox/%3Cca9479fbd5ca4f13913fd072fb00f0d3%40git.apache.org%3E > > > ) > > > > > > On 12 May 2016 at 11:31, Ian Dunlop <[email protected]> wrote: > > > > Hello, > > > > > > > > I think it is git merge --no-ff gale-readme-updates -m "This closes > #1" > > > > Where gale-readme-updates is the local copy of the github pull > request. > > > > Seems to keep all the provenance from the pull request. I'll merge it > > in, > > > > you can shout later if you want ;) > > > > > > > > Cheers, > > > > > > > > Ian > > > > > > > > On 12 May 2016 at 11:28, Ian Dunlop <[email protected]> wrote: > > > > > > > >> Hello, > > > >> > > > >> I was going to merge this then I realised that I've forgotten how I > > did > > > it > > > >> before! What is the best way to merge pull requests? Basically we > want > > > to > > > >> keep the provenance while adding a "This closes #1" type message. I > > > don't > > > >> think we have anything on the site about how to do it. > > > >> > > > >> Cheers, > > > >> > > > >> Ian > > > >> > > > >> On 11 May 2016 at 20:14, Ian Dunlop <[email protected]> wrote: > > > >> > > > >>> Hello, > > > >>> > > > >>> We should get this merged before any release. Thanks Gale. > > > >>> > > > >>> Cheers, > > > >>> > > > >>> Ian > > > >>> > > > >>> On 11 May 2016 at 17:54, Gale Naylor <[email protected]> > > > wrote: > > > >>> > > > >>>> It looks like I still have an open pull request: > > > >>>> https://github.com/apache/incubator-taverna-commandline/pull/1 > > > >>>> > > > >>>> Gale > > > >>>> > > > >>> > > > >>> > > > >> > > > > > > > > > > > > -- > > > Stian Soiland-Reyes > > > Apache Taverna (incubating), Apache Commons RDF (incubating) > > > http://orcid.org/0000-0001-9842-9718 > > > > > >
