taverna-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stian Soiland-Reyes <st...@apache.org>
Subject Re: Pull Request
Date Thu, 12 May 2016 14:04:03 GMT
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" <GaleN@noventussolutions.com> 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 <stain@apache.org>
> 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 <ianwdunlop@gmail.com> 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 <ianwdunlop@gmail.com> 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 <ianwdunlop@gmail.com> wrote:
> > >>
> > >>> Hello,
> > >>>
> > >>> We should get this merged before any release. Thanks Gale.
> > >>>
> > >>> Cheers,
> > >>>
> > >>> Ian
> > >>>
> > >>> On 11 May 2016 at 17:54, Gale Naylor <GaleN@noventussolutions.com>
> > 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
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message