You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltaspike.apache.org by "John D. Ament" <jo...@apache.org> on 2016/07/23 16:04:48 UTC
[DISCUSS] PR Based Contributor Workflow
All,
I put together a first pass PR on an improved contributor workflow that can
leverage github PRs. This is in addition to our existing patch approach.
You can find the PR here, with the changes:
https://github.com/apache/deltaspike/pull/61/files
Using PRs gives us a bit of an advantage:
- We don't lose the original author in the commit
- We can run automated tests prior to the commit being merged in
Please take a look, I'm happy to adjust as needed. I also took the liberty
to replace some of the to-be-retired links (e.g. people.a.o is retiring
soon, mail archives are being moved to pony, ICLA is now PDF based)
John
Re: [DISCUSS] PR Based Contributor Workflow
Posted by "John D. Ament" <jo...@apache.org>.
All,
I've taken the input from Mark and applied it to the changes.
Please review at your convenience. Assuming we're still settled on the
change, I can push this week. (e.g. since most people already voted +1 I'd
like to get a nod from Mark that he's good with the changes).
John
On Wed, Jul 27, 2016 at 6:37 AM John D. Ament <jo...@apache.org> wrote:
> Mark,
>
> Good points.
>
> On Wed, Jul 27, 2016 at 4:54 AM Mark Struberg <st...@yahoo.de.invalid>
> wrote:
>
>> We should add a section that the person who applies the PR to our
>> canonical repo have to verify that the PR only contains commits from the
>> contributor himself. He basically needs to make sure that the contributor
>> doesn't ship too much in the pull request (and thus trashing our code
>> provenance chain).
>>
>
> This note should be added to both PR and Patch sections IMHO. Same issue
> could exist for patches.
>
>
>>
>> ASF committers should not use PRs but directly commit to canonical repo
>> themselves.
>> Of course it's fine to showcase ideas etc on github first. But you can
>> simply cherry pick that over to master and push that to our repo yourself.
>>
>
> To me this is described in the "discussion" workflow, but I can call it
> out a bit clearer. Same should be true of patches though. This would also
> be for DS committers, not necessarily ASF committers (e.g. may be a
> committer on other projects, just not DS, then a PR should be fine as well
> as a patch).
>
>
>>
>> just my .02
>>
>> txs and LieGrue,
>> strub
>>
>>
>>
>>
>>
>> > On Tuesday, 26 July 2016, 1:26, Jason Porter <li...@gmail.com>
>> wrote:
>> > > +1 PRs are much easier to work with, imo.
>> >
>> >
>> > On Sun, Jul 24, 2016 at 1:40 AM, Christian Kaltepoth <
>> christian@kaltepoth.de
>> >> wrote:
>> >
>> >> Hey John,
>> >>
>> >> Great work!
>> >>
>> >> +1 ;)
>> >>
>> >> Christian
>> >>
>> >> 2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
>> >>
>> >> > Hi John,
>> >> >
>> >> > Greate job. I think that we really need to have that. It's much
>> > more easy
>> >> > and cool to work with PR.
>> >> > Easy way to review, easy way to fix changes, the contributor does
>> not
>> >> need
>> >> > to attach a new patch just need to update the PR and we'll have
>> > feedbacks
>> >> > more fast with PR Builder Plugin and comments by line on PR.
>> >> >
>> >> > I prefer this way, totally agree with your PR.
>> >> >
>> >> > +1 :)
>> >> >
>> >> > On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament
>> > <jo...@apache.org>
>> >> > wrote:
>> >> >
>> >> > > All,
>> >> > >
>> >> > > I put together a first pass PR on an improved contributor
>> > workflow that
>> >> > can
>> >> > > leverage github PRs. This is in addition to our existing patch
>> >> approach.
>> >> > >
>> >> > > You can find the PR here, with the changes:
>> >> > > https://github.com/apache/deltaspike/pull/61/files
>> >> > >
>> >> > > Using PRs gives us a bit of an advantage:
>> >> > >
>> >> > > - We don't lose the original author in the commit
>> >> > > - We can run automated tests prior to the commit being merged in
>> >> > >
>> >> > > Please take a look, I'm happy to adjust as needed. I also
>> > took the
>> >> > liberty
>> >> > > to replace some of the to-be-retired links (e.g. people.a.o is
>> > retiring
>> >> > > soon, mail archives are being moved to pony, ICLA is now PDF
>> > based)
>> >> > >
>> >> > > John
>> >> > >
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Daniel Cunha
>> >> > https://twitter.com/dvlc_
>> >> > http://www.tomitribe.com
>> >> > http://www.tomitribe.io
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Christian Kaltepoth
>> >> Blog: http://blog.kaltepoth.de/
>> >> Twitter: http://twitter.com/chkal
>> >> GitHub: https://github.com/chkal
>> >>
>> >
>> >
>> >
>> > --
>> > Jason Porter
>> > http://en.gravatar.com/lightguardjp
>> >
>>
>
Re: [DISCUSS] PR Based Contributor Workflow
Posted by "John D. Ament" <jo...@apache.org>.
Mark,
Good points.
On Wed, Jul 27, 2016 at 4:54 AM Mark Struberg <st...@yahoo.de.invalid>
wrote:
> We should add a section that the person who applies the PR to our
> canonical repo have to verify that the PR only contains commits from the
> contributor himself. He basically needs to make sure that the contributor
> doesn't ship too much in the pull request (and thus trashing our code
> provenance chain).
>
This note should be added to both PR and Patch sections IMHO. Same issue
could exist for patches.
>
> ASF committers should not use PRs but directly commit to canonical repo
> themselves.
> Of course it's fine to showcase ideas etc on github first. But you can
> simply cherry pick that over to master and push that to our repo yourself.
>
To me this is described in the "discussion" workflow, but I can call it out
a bit clearer. Same should be true of patches though. This would also be
for DS committers, not necessarily ASF committers (e.g. may be a committer
on other projects, just not DS, then a PR should be fine as well as a
patch).
>
> just my .02
>
> txs and LieGrue,
> strub
>
>
>
>
>
> > On Tuesday, 26 July 2016, 1:26, Jason Porter <li...@gmail.com>
> wrote:
> > > +1 PRs are much easier to work with, imo.
> >
> >
> > On Sun, Jul 24, 2016 at 1:40 AM, Christian Kaltepoth <
> christian@kaltepoth.de
> >> wrote:
> >
> >> Hey John,
> >>
> >> Great work!
> >>
> >> +1 ;)
> >>
> >> Christian
> >>
> >> 2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
> >>
> >> > Hi John,
> >> >
> >> > Greate job. I think that we really need to have that. It's much
> > more easy
> >> > and cool to work with PR.
> >> > Easy way to review, easy way to fix changes, the contributor does not
> >> need
> >> > to attach a new patch just need to update the PR and we'll have
> > feedbacks
> >> > more fast with PR Builder Plugin and comments by line on PR.
> >> >
> >> > I prefer this way, totally agree with your PR.
> >> >
> >> > +1 :)
> >> >
> >> > On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament
> > <jo...@apache.org>
> >> > wrote:
> >> >
> >> > > All,
> >> > >
> >> > > I put together a first pass PR on an improved contributor
> > workflow that
> >> > can
> >> > > leverage github PRs. This is in addition to our existing patch
> >> approach.
> >> > >
> >> > > You can find the PR here, with the changes:
> >> > > https://github.com/apache/deltaspike/pull/61/files
> >> > >
> >> > > Using PRs gives us a bit of an advantage:
> >> > >
> >> > > - We don't lose the original author in the commit
> >> > > - We can run automated tests prior to the commit being merged in
> >> > >
> >> > > Please take a look, I'm happy to adjust as needed. I also
> > took the
> >> > liberty
> >> > > to replace some of the to-be-retired links (e.g. people.a.o is
> > retiring
> >> > > soon, mail archives are being moved to pony, ICLA is now PDF
> > based)
> >> > >
> >> > > John
> >> > >
> >> >
> >> >
> >> >
> >> > --
> >> > Daniel Cunha
> >> > https://twitter.com/dvlc_
> >> > http://www.tomitribe.com
> >> > http://www.tomitribe.io
> >> >
> >>
> >>
> >>
> >> --
> >> Christian Kaltepoth
> >> Blog: http://blog.kaltepoth.de/
> >> Twitter: http://twitter.com/chkal
> >> GitHub: https://github.com/chkal
> >>
> >
> >
> >
> > --
> > Jason Porter
> > http://en.gravatar.com/lightguardjp
> >
>
Re: [DISCUSS] PR Based Contributor Workflow
Posted by Gerhard Petracek <ge...@gmail.com>.
@mark: +1
regards,
gerhard
2016-07-27 10:53 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:
> We should add a section that the person who applies the PR to our
> canonical repo have to verify that the PR only contains commits from the
> contributor himself. He basically needs to make sure that the contributor
> doesn't ship too much in the pull request (and thus trashing our code
> provenance chain).
>
> ASF committers should not use PRs but directly commit to canonical repo
> themselves.
> Of course it's fine to showcase ideas etc on github first. But you can
> simply cherry pick that over to master and push that to our repo yourself.
>
> just my .02
>
> txs and LieGrue,
> strub
>
>
>
>
>
> > On Tuesday, 26 July 2016, 1:26, Jason Porter <li...@gmail.com>
> wrote:
> > > +1 PRs are much easier to work with, imo.
> >
> >
> > On Sun, Jul 24, 2016 at 1:40 AM, Christian Kaltepoth <
> christian@kaltepoth.de
> >> wrote:
> >
> >> Hey John,
> >>
> >> Great work!
> >>
> >> +1 ;)
> >>
> >> Christian
> >>
> >> 2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
> >>
> >> > Hi John,
> >> >
> >> > Greate job. I think that we really need to have that. It's much
> > more easy
> >> > and cool to work with PR.
> >> > Easy way to review, easy way to fix changes, the contributor does not
> >> need
> >> > to attach a new patch just need to update the PR and we'll have
> > feedbacks
> >> > more fast with PR Builder Plugin and comments by line on PR.
> >> >
> >> > I prefer this way, totally agree with your PR.
> >> >
> >> > +1 :)
> >> >
> >> > On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament
> > <jo...@apache.org>
> >> > wrote:
> >> >
> >> > > All,
> >> > >
> >> > > I put together a first pass PR on an improved contributor
> > workflow that
> >> > can
> >> > > leverage github PRs. This is in addition to our existing patch
> >> approach.
> >> > >
> >> > > You can find the PR here, with the changes:
> >> > > https://github.com/apache/deltaspike/pull/61/files
> >> > >
> >> > > Using PRs gives us a bit of an advantage:
> >> > >
> >> > > - We don't lose the original author in the commit
> >> > > - We can run automated tests prior to the commit being merged in
> >> > >
> >> > > Please take a look, I'm happy to adjust as needed. I also
> > took the
> >> > liberty
> >> > > to replace some of the to-be-retired links (e.g. people.a.o is
> > retiring
> >> > > soon, mail archives are being moved to pony, ICLA is now PDF
> > based)
> >> > >
> >> > > John
> >> > >
> >> >
> >> >
> >> >
> >> > --
> >> > Daniel Cunha
> >> > https://twitter.com/dvlc_
> >> > http://www.tomitribe.com
> >> > http://www.tomitribe.io
> >> >
> >>
> >>
> >>
> >> --
> >> Christian Kaltepoth
> >> Blog: http://blog.kaltepoth.de/
> >> Twitter: http://twitter.com/chkal
> >> GitHub: https://github.com/chkal
> >>
> >
> >
> >
> > --
> > Jason Porter
> > http://en.gravatar.com/lightguardjp
> >
>
Re: [DISCUSS] PR Based Contributor Workflow
Posted by Mark Struberg <st...@yahoo.de.INVALID>.
We should add a section that the person who applies the PR to our canonical repo have to verify that the PR only contains commits from the contributor himself. He basically needs to make sure that the contributor doesn't ship too much in the pull request (and thus trashing our code provenance chain).
ASF committers should not use PRs but directly commit to canonical repo themselves.
Of course it's fine to showcase ideas etc on github first. But you can simply cherry pick that over to master and push that to our repo yourself.
just my .02
txs and LieGrue,
strub
> On Tuesday, 26 July 2016, 1:26, Jason Porter <li...@gmail.com> wrote:
> > +1 PRs are much easier to work with, imo.
>
>
> On Sun, Jul 24, 2016 at 1:40 AM, Christian Kaltepoth <christian@kaltepoth.de
>> wrote:
>
>> Hey John,
>>
>> Great work!
>>
>> +1 ;)
>>
>> Christian
>>
>> 2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
>>
>> > Hi John,
>> >
>> > Greate job. I think that we really need to have that. It's much
> more easy
>> > and cool to work with PR.
>> > Easy way to review, easy way to fix changes, the contributor does not
>> need
>> > to attach a new patch just need to update the PR and we'll have
> feedbacks
>> > more fast with PR Builder Plugin and comments by line on PR.
>> >
>> > I prefer this way, totally agree with your PR.
>> >
>> > +1 :)
>> >
>> > On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament
> <jo...@apache.org>
>> > wrote:
>> >
>> > > All,
>> > >
>> > > I put together a first pass PR on an improved contributor
> workflow that
>> > can
>> > > leverage github PRs. This is in addition to our existing patch
>> approach.
>> > >
>> > > You can find the PR here, with the changes:
>> > > https://github.com/apache/deltaspike/pull/61/files
>> > >
>> > > Using PRs gives us a bit of an advantage:
>> > >
>> > > - We don't lose the original author in the commit
>> > > - We can run automated tests prior to the commit being merged in
>> > >
>> > > Please take a look, I'm happy to adjust as needed. I also
> took the
>> > liberty
>> > > to replace some of the to-be-retired links (e.g. people.a.o is
> retiring
>> > > soon, mail archives are being moved to pony, ICLA is now PDF
> based)
>> > >
>> > > John
>> > >
>> >
>> >
>> >
>> > --
>> > Daniel Cunha
>> > https://twitter.com/dvlc_
>> > http://www.tomitribe.com
>> > http://www.tomitribe.io
>> >
>>
>>
>>
>> --
>> Christian Kaltepoth
>> Blog: http://blog.kaltepoth.de/
>> Twitter: http://twitter.com/chkal
>> GitHub: https://github.com/chkal
>>
>
>
>
> --
> Jason Porter
> http://en.gravatar.com/lightguardjp
>
Re: [DISCUSS] PR Based Contributor Workflow
Posted by Jason Porter <li...@gmail.com>.
+1 PRs are much easier to work with, imo.
On Sun, Jul 24, 2016 at 1:40 AM, Christian Kaltepoth <christian@kaltepoth.de
> wrote:
> Hey John,
>
> Great work!
>
> +1 ;)
>
> Christian
>
> 2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
>
> > Hi John,
> >
> > Greate job. I think that we really need to have that. It's much more easy
> > and cool to work with PR.
> > Easy way to review, easy way to fix changes, the contributor does not
> need
> > to attach a new patch just need to update the PR and we'll have feedbacks
> > more fast with PR Builder Plugin and comments by line on PR.
> >
> > I prefer this way, totally agree with your PR.
> >
> > +1 :)
> >
> > On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament <jo...@apache.org>
> > wrote:
> >
> > > All,
> > >
> > > I put together a first pass PR on an improved contributor workflow that
> > can
> > > leverage github PRs. This is in addition to our existing patch
> approach.
> > >
> > > You can find the PR here, with the changes:
> > > https://github.com/apache/deltaspike/pull/61/files
> > >
> > > Using PRs gives us a bit of an advantage:
> > >
> > > - We don't lose the original author in the commit
> > > - We can run automated tests prior to the commit being merged in
> > >
> > > Please take a look, I'm happy to adjust as needed. I also took the
> > liberty
> > > to replace some of the to-be-retired links (e.g. people.a.o is retiring
> > > soon, mail archives are being moved to pony, ICLA is now PDF based)
> > >
> > > John
> > >
> >
> >
> >
> > --
> > Daniel Cunha
> > https://twitter.com/dvlc_
> > http://www.tomitribe.com
> > http://www.tomitribe.io
> >
>
>
>
> --
> Christian Kaltepoth
> Blog: http://blog.kaltepoth.de/
> Twitter: http://twitter.com/chkal
> GitHub: https://github.com/chkal
>
--
Jason Porter
http://en.gravatar.com/lightguardjp
Re: [DISCUSS] PR Based Contributor Workflow
Posted by Christian Kaltepoth <ch...@kaltepoth.de>.
Hey John,
Great work!
+1 ;)
Christian
2016-07-23 18:14 GMT+02:00 Daniel Cunha <da...@gmail.com>:
> Hi John,
>
> Greate job. I think that we really need to have that. It's much more easy
> and cool to work with PR.
> Easy way to review, easy way to fix changes, the contributor does not need
> to attach a new patch just need to update the PR and we'll have feedbacks
> more fast with PR Builder Plugin and comments by line on PR.
>
> I prefer this way, totally agree with your PR.
>
> +1 :)
>
> On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament <jo...@apache.org>
> wrote:
>
> > All,
> >
> > I put together a first pass PR on an improved contributor workflow that
> can
> > leverage github PRs. This is in addition to our existing patch approach.
> >
> > You can find the PR here, with the changes:
> > https://github.com/apache/deltaspike/pull/61/files
> >
> > Using PRs gives us a bit of an advantage:
> >
> > - We don't lose the original author in the commit
> > - We can run automated tests prior to the commit being merged in
> >
> > Please take a look, I'm happy to adjust as needed. I also took the
> liberty
> > to replace some of the to-be-retired links (e.g. people.a.o is retiring
> > soon, mail archives are being moved to pony, ICLA is now PDF based)
> >
> > John
> >
>
>
>
> --
> Daniel Cunha
> https://twitter.com/dvlc_
> http://www.tomitribe.com
> http://www.tomitribe.io
>
--
Christian Kaltepoth
Blog: http://blog.kaltepoth.de/
Twitter: http://twitter.com/chkal
GitHub: https://github.com/chkal
Re: [DISCUSS] PR Based Contributor Workflow
Posted by Daniel Cunha <da...@gmail.com>.
Hi John,
Greate job. I think that we really need to have that. It's much more easy
and cool to work with PR.
Easy way to review, easy way to fix changes, the contributor does not need
to attach a new patch just need to update the PR and we'll have feedbacks
more fast with PR Builder Plugin and comments by line on PR.
I prefer this way, totally agree with your PR.
+1 :)
On Sat, Jul 23, 2016 at 1:04 PM, John D. Ament <jo...@apache.org>
wrote:
> All,
>
> I put together a first pass PR on an improved contributor workflow that can
> leverage github PRs. This is in addition to our existing patch approach.
>
> You can find the PR here, with the changes:
> https://github.com/apache/deltaspike/pull/61/files
>
> Using PRs gives us a bit of an advantage:
>
> - We don't lose the original author in the commit
> - We can run automated tests prior to the commit being merged in
>
> Please take a look, I'm happy to adjust as needed. I also took the liberty
> to replace some of the to-be-retired links (e.g. people.a.o is retiring
> soon, mail archives are being moved to pony, ICLA is now PDF based)
>
> John
>
--
Daniel Cunha
https://twitter.com/dvlc_
http://www.tomitribe.com
http://www.tomitribe.io