You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Yi Pan <ni...@gmail.com> on 2016/02/19 00:53:07 UTC

[DISCUSS] Moving to github/pull-request for code review and check-in

Hi, all,

I want to start the discussion on our code review/commit process.

I felt that our code review and check-in process is a little bit
cumbersome:
- developers need to create RBs and attach diff to JIRA
- committers need to review RBs, dowload diff and apply, then push.

It would be much lighter if we take the pull request only approach, as
Kafka already converted to:
- for the developers, the only thing needed is to open a pull request.
- for committers, review and apply patch is from the same PR and merge can
be done directly on remote git repo.

Of course, there might be some hookup scripts that we will need to link
JIRA w/ pull request in github, which Kafka already does. Any comments and
feedbacks are welcome!

Thanks!

-Yi

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Edi Bice <ed...@gmail.com>.
Yay!

> On Feb 18, 2016, at 7:25 PM, Navina Ramesh <nr...@linkedin.com.INVALID> wrote:
> 
> +1
> 
> Haven't tried any contribution with pull requests. But sounds simpler than
> attaching the patch to JIRA.
> 
> Navina
> 
>> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <ja...@gmail.com> wrote:
>> 
>> +1
>> 
>> As a relatively new contributor to Samza, I've certainly felt the current
>> process was overly-complicated.
>> 
>>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
>>> 
>>> Hi, all,
>>> 
>>> I want to start the discussion on our code review/commit process.
>>> 
>>> I felt that our code review and check-in process is a little bit
>>> cumbersome:
>>> - developers need to create RBs and attach diff to JIRA
>>> - committers need to review RBs, dowload diff and apply, then push.
>>> 
>>> It would be much lighter if we take the pull request only approach, as
>>> Kafka already converted to:
>>> - for the developers, the only thing needed is to open a pull request.
>>> - for committers, review and apply patch is from the same PR and merge
>> can
>>> be done directly on remote git repo.
>>> 
>>> Of course, there might be some hookup scripts that we will need to link
>>> JIRA w/ pull request in github, which Kafka already does. Any comments
>> and
>>> feedbacks are welcome!
>>> 
>>> Thanks!
>>> 
>>> -Yi
> 
> 
> 
> -- 
> Navina R.

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Chinmay Soman <ch...@gmail.com>.
+1  for pull requests.

On Fri, Feb 19, 2016 at 3:12 PM, Yi Pan <ni...@gmail.com> wrote:

> Hi, Julian,
>
> Thanks for the input. It is a good point that directly merge on github may
> result in non-linear history in the master branch. I just checked the
> kafka-merge-pr.py script Kafka uses to merge the PRs to master and the
> basic workflow it implements is the same as what we manually enforce as for
> today:
> 1) checkout target branch and PR branch
> 2) set the local workspace to the target branch (i.e. "git checkout
> ${target_branch}")
> 3) run "git merge --squash ${PR_branch}"
> 4) if merge successfully, "git commit" and "git push"
>
> Essentially the above steps are the ones we documented in our wiki for
> committer workflow and it works well for Kafka. We can adopt that script in
> Samza committer workflow as well.
>
> Thanks!
>
> -Yi
>
> On Fri, Feb 19, 2016 at 2:44 PM, Julian Hyde <jh...@apache.org> wrote:
>
> > PRs have worked well for us in Calcite.
> >
> > We still accept patches, if contributors are adamant, but it’s unusual.
> We
> > don’t use RB.
> >
> > We (or I) haven’t managed to fully automate submission. I pull down to my
> > sandbox, rebase, and merge --ff-only, because in Calcite (as I think in
> > Samza) our policy it to rebase onto the master rather than merge[1].
> >
> > I also need to add a commit comment ‘Close apache/calcite#nnn’ to tell
> the
> > ASF bot to close the PR.
> >
> > And, if you’re a committer, you need to be firm about getting a PR. You
> > can’t accept a contribution which is just someone pasting the URL of
> their
> > github branch into a JIRA. For IP hygiene, they must create a PR.
> >
> > Even with all that, I strongly recommend Samza moving to PRs.
> >
> > Julian
> >
> > [1] https://www.atlassian.com/git/tutorials/merging-vs-rebasing/
> >
> > > On Feb 19, 2016, at 8:43 AM, Jagadish Venkatraman <
> > jagadish1989@gmail.com> wrote:
> > >
> > > +1 attaching patches to jira is heavy weight.
> > >
> > > On Friday, February 19, 2016, Yan Fang <ya...@gmail.com> wrote:
> > >
> > >> +1.
> > >>
> > >> Though I am familiar with the current way, still think the pull
> requests
> > >> are simpler.
> > >>
> > >> Cheers,
> > >>
> > >> Fang, Yan
> > >> yanfang724@gmail.com <javascript:;>
> > >>
> > >> On Fri, Feb 19, 2016 at 11:10 AM, Milinda Pathirage <
> > mpathira@umail.iu.edu
> > >> <javascript:;>>
> > >> wrote:
> > >>
> > >>> +1. Calcite uses pull requests for contributions from non-committers
> > and
> > >>> according to my experience with Calcite, pull requests are easier
> than
> > >> the
> > >>> current approach we follow in Samza.
> > >>>
> > >>> Milinda
> > >>>
> > >>> On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <
> roger.hoover@gmail.com
> > >> <javascript:;>>
> > >>> wrote:
> > >>>
> > >>>> +1 - Thanks for bringing this up, Yi.  I've done it both ways and
> feel
> > >>>> pull requests are much easier.
> > >>>>
> > >>>> Sent from my iPhone
> > >>>>
> > >>>>> On Feb 18, 2016, at 4:25 PM, Navina Ramesh
> > >>> <nr...@linkedin.com.INVALID>
> > >>>> wrote:
> > >>>>>
> > >>>>> +1
> > >>>>>
> > >>>>> Haven't tried any contribution with pull requests. But sounds
> simpler
> > >>>> than
> > >>>>> attaching the patch to JIRA.
> > >>>>>
> > >>>>> Navina
> > >>>>>
> > >>>>>> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <jacob.maes@gmail.com
> > >> <javascript:;>>
> > >>>> wrote:
> > >>>>>>
> > >>>>>> +1
> > >>>>>>
> > >>>>>> As a relatively new contributor to Samza, I've certainly felt the
> > >>>> current
> > >>>>>> process was overly-complicated.
> > >>>>>>
> > >>>>>>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <nickpan47@gmail.com
> > >> <javascript:;>> wrote:
> > >>>>>>>
> > >>>>>>> Hi, all,
> > >>>>>>>
> > >>>>>>> I want to start the discussion on our code review/commit process.
> > >>>>>>>
> > >>>>>>> I felt that our code review and check-in process is a little bit
> > >>>>>>> cumbersome:
> > >>>>>>> - developers need to create RBs and attach diff to JIRA
> > >>>>>>> - committers need to review RBs, dowload diff and apply, then
> push.
> > >>>>>>>
> > >>>>>>> It would be much lighter if we take the pull request only
> approach,
> > >>> as
> > >>>>>>> Kafka already converted to:
> > >>>>>>> - for the developers, the only thing needed is to open a pull
> > >>> request.
> > >>>>>>> - for committers, review and apply patch is from the same PR and
> > >>> merge
> > >>>>>> can
> > >>>>>>> be done directly on remote git repo.
> > >>>>>>>
> > >>>>>>> Of course, there might be some hookup scripts that we will need
> to
> > >>> link
> > >>>>>>> JIRA w/ pull request in github, which Kafka already does. Any
> > >>> comments
> > >>>>>> and
> > >>>>>>> feedbacks are welcome!
> > >>>>>>>
> > >>>>>>> Thanks!
> > >>>>>>>
> > >>>>>>> -Yi
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> --
> > >>>>> Navina R.
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>> --
> > >>> Milinda Pathirage
> > >>>
> > >>> PhD Student | Research Assistant
> > >>> School of Informatics and Computing | Data to Insight Center
> > >>> Indiana University
> > >>>
> > >>> twitter: milindalakmal
> > >>> skype: milinda.pathirage
> > >>> blog: http://milinda.pathirage.org
> > >>>
> > >>
> > >
> > >
> > > --
> > > Sent from my iphone.
> >
> >
>



-- 
Thanks and regards

Chinmay Soman

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Yi Pan <ni...@gmail.com>.
Hi, Julian,

Thanks for the input. It is a good point that directly merge on github may
result in non-linear history in the master branch. I just checked the
kafka-merge-pr.py script Kafka uses to merge the PRs to master and the
basic workflow it implements is the same as what we manually enforce as for
today:
1) checkout target branch and PR branch
2) set the local workspace to the target branch (i.e. "git checkout
${target_branch}")
3) run "git merge --squash ${PR_branch}"
4) if merge successfully, "git commit" and "git push"

Essentially the above steps are the ones we documented in our wiki for
committer workflow and it works well for Kafka. We can adopt that script in
Samza committer workflow as well.

Thanks!

-Yi

On Fri, Feb 19, 2016 at 2:44 PM, Julian Hyde <jh...@apache.org> wrote:

> PRs have worked well for us in Calcite.
>
> We still accept patches, if contributors are adamant, but it’s unusual. We
> don’t use RB.
>
> We (or I) haven’t managed to fully automate submission. I pull down to my
> sandbox, rebase, and merge --ff-only, because in Calcite (as I think in
> Samza) our policy it to rebase onto the master rather than merge[1].
>
> I also need to add a commit comment ‘Close apache/calcite#nnn’ to tell the
> ASF bot to close the PR.
>
> And, if you’re a committer, you need to be firm about getting a PR. You
> can’t accept a contribution which is just someone pasting the URL of their
> github branch into a JIRA. For IP hygiene, they must create a PR.
>
> Even with all that, I strongly recommend Samza moving to PRs.
>
> Julian
>
> [1] https://www.atlassian.com/git/tutorials/merging-vs-rebasing/
>
> > On Feb 19, 2016, at 8:43 AM, Jagadish Venkatraman <
> jagadish1989@gmail.com> wrote:
> >
> > +1 attaching patches to jira is heavy weight.
> >
> > On Friday, February 19, 2016, Yan Fang <ya...@gmail.com> wrote:
> >
> >> +1.
> >>
> >> Though I am familiar with the current way, still think the pull requests
> >> are simpler.
> >>
> >> Cheers,
> >>
> >> Fang, Yan
> >> yanfang724@gmail.com <javascript:;>
> >>
> >> On Fri, Feb 19, 2016 at 11:10 AM, Milinda Pathirage <
> mpathira@umail.iu.edu
> >> <javascript:;>>
> >> wrote:
> >>
> >>> +1. Calcite uses pull requests for contributions from non-committers
> and
> >>> according to my experience with Calcite, pull requests are easier than
> >> the
> >>> current approach we follow in Samza.
> >>>
> >>> Milinda
> >>>
> >>> On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <roger.hoover@gmail.com
> >> <javascript:;>>
> >>> wrote:
> >>>
> >>>> +1 - Thanks for bringing this up, Yi.  I've done it both ways and feel
> >>>> pull requests are much easier.
> >>>>
> >>>> Sent from my iPhone
> >>>>
> >>>>> On Feb 18, 2016, at 4:25 PM, Navina Ramesh
> >>> <nr...@linkedin.com.INVALID>
> >>>> wrote:
> >>>>>
> >>>>> +1
> >>>>>
> >>>>> Haven't tried any contribution with pull requests. But sounds simpler
> >>>> than
> >>>>> attaching the patch to JIRA.
> >>>>>
> >>>>> Navina
> >>>>>
> >>>>>> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <jacob.maes@gmail.com
> >> <javascript:;>>
> >>>> wrote:
> >>>>>>
> >>>>>> +1
> >>>>>>
> >>>>>> As a relatively new contributor to Samza, I've certainly felt the
> >>>> current
> >>>>>> process was overly-complicated.
> >>>>>>
> >>>>>>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <nickpan47@gmail.com
> >> <javascript:;>> wrote:
> >>>>>>>
> >>>>>>> Hi, all,
> >>>>>>>
> >>>>>>> I want to start the discussion on our code review/commit process.
> >>>>>>>
> >>>>>>> I felt that our code review and check-in process is a little bit
> >>>>>>> cumbersome:
> >>>>>>> - developers need to create RBs and attach diff to JIRA
> >>>>>>> - committers need to review RBs, dowload diff and apply, then push.
> >>>>>>>
> >>>>>>> It would be much lighter if we take the pull request only approach,
> >>> as
> >>>>>>> Kafka already converted to:
> >>>>>>> - for the developers, the only thing needed is to open a pull
> >>> request.
> >>>>>>> - for committers, review and apply patch is from the same PR and
> >>> merge
> >>>>>> can
> >>>>>>> be done directly on remote git repo.
> >>>>>>>
> >>>>>>> Of course, there might be some hookup scripts that we will need to
> >>> link
> >>>>>>> JIRA w/ pull request in github, which Kafka already does. Any
> >>> comments
> >>>>>> and
> >>>>>>> feedbacks are welcome!
> >>>>>>>
> >>>>>>> Thanks!
> >>>>>>>
> >>>>>>> -Yi
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Navina R.
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Milinda Pathirage
> >>>
> >>> PhD Student | Research Assistant
> >>> School of Informatics and Computing | Data to Insight Center
> >>> Indiana University
> >>>
> >>> twitter: milindalakmal
> >>> skype: milinda.pathirage
> >>> blog: http://milinda.pathirage.org
> >>>
> >>
> >
> >
> > --
> > Sent from my iphone.
>
>

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Julian Hyde <jh...@apache.org>.
PRs have worked well for us in Calcite.

We still accept patches, if contributors are adamant, but it’s unusual. We don’t use RB.

We (or I) haven’t managed to fully automate submission. I pull down to my sandbox, rebase, and merge --ff-only, because in Calcite (as I think in Samza) our policy it to rebase onto the master rather than merge[1].

I also need to add a commit comment ‘Close apache/calcite#nnn’ to tell the ASF bot to close the PR.

And, if you’re a committer, you need to be firm about getting a PR. You can’t accept a contribution which is just someone pasting the URL of their github branch into a JIRA. For IP hygiene, they must create a PR.

Even with all that, I strongly recommend Samza moving to PRs.

Julian

[1] https://www.atlassian.com/git/tutorials/merging-vs-rebasing/

> On Feb 19, 2016, at 8:43 AM, Jagadish Venkatraman <ja...@gmail.com> wrote:
> 
> +1 attaching patches to jira is heavy weight.
> 
> On Friday, February 19, 2016, Yan Fang <ya...@gmail.com> wrote:
> 
>> +1.
>> 
>> Though I am familiar with the current way, still think the pull requests
>> are simpler.
>> 
>> Cheers,
>> 
>> Fang, Yan
>> yanfang724@gmail.com <javascript:;>
>> 
>> On Fri, Feb 19, 2016 at 11:10 AM, Milinda Pathirage <mpathira@umail.iu.edu
>> <javascript:;>>
>> wrote:
>> 
>>> +1. Calcite uses pull requests for contributions from non-committers and
>>> according to my experience with Calcite, pull requests are easier than
>> the
>>> current approach we follow in Samza.
>>> 
>>> Milinda
>>> 
>>> On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <roger.hoover@gmail.com
>> <javascript:;>>
>>> wrote:
>>> 
>>>> +1 - Thanks for bringing this up, Yi.  I've done it both ways and feel
>>>> pull requests are much easier.
>>>> 
>>>> Sent from my iPhone
>>>> 
>>>>> On Feb 18, 2016, at 4:25 PM, Navina Ramesh
>>> <nr...@linkedin.com.INVALID>
>>>> wrote:
>>>>> 
>>>>> +1
>>>>> 
>>>>> Haven't tried any contribution with pull requests. But sounds simpler
>>>> than
>>>>> attaching the patch to JIRA.
>>>>> 
>>>>> Navina
>>>>> 
>>>>>> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <jacob.maes@gmail.com
>> <javascript:;>>
>>>> wrote:
>>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> As a relatively new contributor to Samza, I've certainly felt the
>>>> current
>>>>>> process was overly-complicated.
>>>>>> 
>>>>>>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <nickpan47@gmail.com
>> <javascript:;>> wrote:
>>>>>>> 
>>>>>>> Hi, all,
>>>>>>> 
>>>>>>> I want to start the discussion on our code review/commit process.
>>>>>>> 
>>>>>>> I felt that our code review and check-in process is a little bit
>>>>>>> cumbersome:
>>>>>>> - developers need to create RBs and attach diff to JIRA
>>>>>>> - committers need to review RBs, dowload diff and apply, then push.
>>>>>>> 
>>>>>>> It would be much lighter if we take the pull request only approach,
>>> as
>>>>>>> Kafka already converted to:
>>>>>>> - for the developers, the only thing needed is to open a pull
>>> request.
>>>>>>> - for committers, review and apply patch is from the same PR and
>>> merge
>>>>>> can
>>>>>>> be done directly on remote git repo.
>>>>>>> 
>>>>>>> Of course, there might be some hookup scripts that we will need to
>>> link
>>>>>>> JIRA w/ pull request in github, which Kafka already does. Any
>>> comments
>>>>>> and
>>>>>>> feedbacks are welcome!
>>>>>>> 
>>>>>>> Thanks!
>>>>>>> 
>>>>>>> -Yi
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Navina R.
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Milinda Pathirage
>>> 
>>> PhD Student | Research Assistant
>>> School of Informatics and Computing | Data to Insight Center
>>> Indiana University
>>> 
>>> twitter: milindalakmal
>>> skype: milinda.pathirage
>>> blog: http://milinda.pathirage.org
>>> 
>> 
> 
> 
> -- 
> Sent from my iphone.


Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Jagadish Venkatraman <ja...@gmail.com>.
+1 attaching patches to jira is heavy weight.

On Friday, February 19, 2016, Yan Fang <ya...@gmail.com> wrote:

> +1.
>
> Though I am familiar with the current way, still think the pull requests
> are simpler.
>
> Cheers,
>
> Fang, Yan
> yanfang724@gmail.com <javascript:;>
>
> On Fri, Feb 19, 2016 at 11:10 AM, Milinda Pathirage <mpathira@umail.iu.edu
> <javascript:;>>
> wrote:
>
> > +1. Calcite uses pull requests for contributions from non-committers and
> > according to my experience with Calcite, pull requests are easier than
> the
> > current approach we follow in Samza.
> >
> > Milinda
> >
> > On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <roger.hoover@gmail.com
> <javascript:;>>
> > wrote:
> >
> > > +1 - Thanks for bringing this up, Yi.  I've done it both ways and feel
> > > pull requests are much easier.
> > >
> > > Sent from my iPhone
> > >
> > > > On Feb 18, 2016, at 4:25 PM, Navina Ramesh
> > <nr...@linkedin.com.INVALID>
> > > wrote:
> > > >
> > > > +1
> > > >
> > > > Haven't tried any contribution with pull requests. But sounds simpler
> > > than
> > > > attaching the patch to JIRA.
> > > >
> > > > Navina
> > > >
> > > >> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <jacob.maes@gmail.com
> <javascript:;>>
> > > wrote:
> > > >>
> > > >> +1
> > > >>
> > > >> As a relatively new contributor to Samza, I've certainly felt the
> > > current
> > > >> process was overly-complicated.
> > > >>
> > > >>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <nickpan47@gmail.com
> <javascript:;>> wrote:
> > > >>>
> > > >>> Hi, all,
> > > >>>
> > > >>> I want to start the discussion on our code review/commit process.
> > > >>>
> > > >>> I felt that our code review and check-in process is a little bit
> > > >>> cumbersome:
> > > >>> - developers need to create RBs and attach diff to JIRA
> > > >>> - committers need to review RBs, dowload diff and apply, then push.
> > > >>>
> > > >>> It would be much lighter if we take the pull request only approach,
> > as
> > > >>> Kafka already converted to:
> > > >>> - for the developers, the only thing needed is to open a pull
> > request.
> > > >>> - for committers, review and apply patch is from the same PR and
> > merge
> > > >> can
> > > >>> be done directly on remote git repo.
> > > >>>
> > > >>> Of course, there might be some hookup scripts that we will need to
> > link
> > > >>> JIRA w/ pull request in github, which Kafka already does. Any
> > comments
> > > >> and
> > > >>> feedbacks are welcome!
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>> -Yi
> > > >
> > > >
> > > >
> > > > --
> > > > Navina R.
> > >
> >
> >
> >
> > --
> > Milinda Pathirage
> >
> > PhD Student | Research Assistant
> > School of Informatics and Computing | Data to Insight Center
> > Indiana University
> >
> > twitter: milindalakmal
> > skype: milinda.pathirage
> > blog: http://milinda.pathirage.org
> >
>


-- 
Sent from my iphone.

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Yan Fang <ya...@gmail.com>.
+1.

Though I am familiar with the current way, still think the pull requests
are simpler.

Cheers,

Fang, Yan
yanfang724@gmail.com

On Fri, Feb 19, 2016 at 11:10 AM, Milinda Pathirage <mp...@umail.iu.edu>
wrote:

> +1. Calcite uses pull requests for contributions from non-committers and
> according to my experience with Calcite, pull requests are easier than the
> current approach we follow in Samza.
>
> Milinda
>
> On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <ro...@gmail.com>
> wrote:
>
> > +1 - Thanks for bringing this up, Yi.  I've done it both ways and feel
> > pull requests are much easier.
> >
> > Sent from my iPhone
> >
> > > On Feb 18, 2016, at 4:25 PM, Navina Ramesh
> <nr...@linkedin.com.INVALID>
> > wrote:
> > >
> > > +1
> > >
> > > Haven't tried any contribution with pull requests. But sounds simpler
> > than
> > > attaching the patch to JIRA.
> > >
> > > Navina
> > >
> > >> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <ja...@gmail.com>
> > wrote:
> > >>
> > >> +1
> > >>
> > >> As a relatively new contributor to Samza, I've certainly felt the
> > current
> > >> process was overly-complicated.
> > >>
> > >>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
> > >>>
> > >>> Hi, all,
> > >>>
> > >>> I want to start the discussion on our code review/commit process.
> > >>>
> > >>> I felt that our code review and check-in process is a little bit
> > >>> cumbersome:
> > >>> - developers need to create RBs and attach diff to JIRA
> > >>> - committers need to review RBs, dowload diff and apply, then push.
> > >>>
> > >>> It would be much lighter if we take the pull request only approach,
> as
> > >>> Kafka already converted to:
> > >>> - for the developers, the only thing needed is to open a pull
> request.
> > >>> - for committers, review and apply patch is from the same PR and
> merge
> > >> can
> > >>> be done directly on remote git repo.
> > >>>
> > >>> Of course, there might be some hookup scripts that we will need to
> link
> > >>> JIRA w/ pull request in github, which Kafka already does. Any
> comments
> > >> and
> > >>> feedbacks are welcome!
> > >>>
> > >>> Thanks!
> > >>>
> > >>> -Yi
> > >
> > >
> > >
> > > --
> > > Navina R.
> >
>
>
>
> --
> Milinda Pathirage
>
> PhD Student | Research Assistant
> School of Informatics and Computing | Data to Insight Center
> Indiana University
>
> twitter: milindalakmal
> skype: milinda.pathirage
> blog: http://milinda.pathirage.org
>

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Milinda Pathirage <mp...@umail.iu.edu>.
+1. Calcite uses pull requests for contributions from non-committers and
according to my experience with Calcite, pull requests are easier than the
current approach we follow in Samza.

Milinda

On Thu, Feb 18, 2016 at 9:09 PM, Roger Hoover <ro...@gmail.com>
wrote:

> +1 - Thanks for bringing this up, Yi.  I've done it both ways and feel
> pull requests are much easier.
>
> Sent from my iPhone
>
> > On Feb 18, 2016, at 4:25 PM, Navina Ramesh <nr...@linkedin.com.INVALID>
> wrote:
> >
> > +1
> >
> > Haven't tried any contribution with pull requests. But sounds simpler
> than
> > attaching the patch to JIRA.
> >
> > Navina
> >
> >> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <ja...@gmail.com>
> wrote:
> >>
> >> +1
> >>
> >> As a relatively new contributor to Samza, I've certainly felt the
> current
> >> process was overly-complicated.
> >>
> >>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
> >>>
> >>> Hi, all,
> >>>
> >>> I want to start the discussion on our code review/commit process.
> >>>
> >>> I felt that our code review and check-in process is a little bit
> >>> cumbersome:
> >>> - developers need to create RBs and attach diff to JIRA
> >>> - committers need to review RBs, dowload diff and apply, then push.
> >>>
> >>> It would be much lighter if we take the pull request only approach, as
> >>> Kafka already converted to:
> >>> - for the developers, the only thing needed is to open a pull request.
> >>> - for committers, review and apply patch is from the same PR and merge
> >> can
> >>> be done directly on remote git repo.
> >>>
> >>> Of course, there might be some hookup scripts that we will need to link
> >>> JIRA w/ pull request in github, which Kafka already does. Any comments
> >> and
> >>> feedbacks are welcome!
> >>>
> >>> Thanks!
> >>>
> >>> -Yi
> >
> >
> >
> > --
> > Navina R.
>



-- 
Milinda Pathirage

PhD Student | Research Assistant
School of Informatics and Computing | Data to Insight Center
Indiana University

twitter: milindalakmal
skype: milinda.pathirage
blog: http://milinda.pathirage.org

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Roger Hoover <ro...@gmail.com>.
+1 - Thanks for bringing this up, Yi.  I've done it both ways and feel pull requests are much easier.

Sent from my iPhone

> On Feb 18, 2016, at 4:25 PM, Navina Ramesh <nr...@linkedin.com.INVALID> wrote:
> 
> +1
> 
> Haven't tried any contribution with pull requests. But sounds simpler than
> attaching the patch to JIRA.
> 
> Navina
> 
>> On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <ja...@gmail.com> wrote:
>> 
>> +1
>> 
>> As a relatively new contributor to Samza, I've certainly felt the current
>> process was overly-complicated.
>> 
>>> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
>>> 
>>> Hi, all,
>>> 
>>> I want to start the discussion on our code review/commit process.
>>> 
>>> I felt that our code review and check-in process is a little bit
>>> cumbersome:
>>> - developers need to create RBs and attach diff to JIRA
>>> - committers need to review RBs, dowload diff and apply, then push.
>>> 
>>> It would be much lighter if we take the pull request only approach, as
>>> Kafka already converted to:
>>> - for the developers, the only thing needed is to open a pull request.
>>> - for committers, review and apply patch is from the same PR and merge
>> can
>>> be done directly on remote git repo.
>>> 
>>> Of course, there might be some hookup scripts that we will need to link
>>> JIRA w/ pull request in github, which Kafka already does. Any comments
>> and
>>> feedbacks are welcome!
>>> 
>>> Thanks!
>>> 
>>> -Yi
> 
> 
> 
> -- 
> Navina R.

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Navina Ramesh <nr...@linkedin.com.INVALID>.
+1

Haven't tried any contribution with pull requests. But sounds simpler than
attaching the patch to JIRA.

Navina

On Thu, Feb 18, 2016 at 4:01 PM, Jacob Maes <ja...@gmail.com> wrote:

> +1
>
> As a relatively new contributor to Samza, I've certainly felt the current
> process was overly-complicated.
>
> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
>
> > Hi, all,
> >
> > I want to start the discussion on our code review/commit process.
> >
> > I felt that our code review and check-in process is a little bit
> > cumbersome:
> > - developers need to create RBs and attach diff to JIRA
> > - committers need to review RBs, dowload diff and apply, then push.
> >
> > It would be much lighter if we take the pull request only approach, as
> > Kafka already converted to:
> > - for the developers, the only thing needed is to open a pull request.
> > - for committers, review and apply patch is from the same PR and merge
> can
> > be done directly on remote git repo.
> >
> > Of course, there might be some hookup scripts that we will need to link
> > JIRA w/ pull request in github, which Kafka already does. Any comments
> and
> > feedbacks are welcome!
> >
> > Thanks!
> >
> > -Yi
> >
>



-- 
Navina R.

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Jacob Maes <ja...@gmail.com>.
+1

As a relatively new contributor to Samza, I've certainly felt the current
process was overly-complicated.

On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:

> Hi, all,
>
> I want to start the discussion on our code review/commit process.
>
> I felt that our code review and check-in process is a little bit
> cumbersome:
> - developers need to create RBs and attach diff to JIRA
> - committers need to review RBs, dowload diff and apply, then push.
>
> It would be much lighter if we take the pull request only approach, as
> Kafka already converted to:
> - for the developers, the only thing needed is to open a pull request.
> - for committers, review and apply patch is from the same PR and merge can
> be done directly on remote git repo.
>
> Of course, there might be some hookup scripts that we will need to link
> JIRA w/ pull request in github, which Kafka already does. Any comments and
> feedbacks are welcome!
>
> Thanks!
>
> -Yi
>

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Yi Pan <ni...@gmail.com>.
Thanks for all the +1s! I have created
https://issues.apache.org/jira/browse/SAMZA-880 to track it.

Thanks!

-Yi

On Wed, Feb 24, 2016 at 5:31 PM, Boris Shkolnik <bo...@gmail.com> wrote:

> +1 for pull requests.
>
> On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:
>
> > Hi, all,
> >
> > I want to start the discussion on our code review/commit process.
> >
> > I felt that our code review and check-in process is a little bit
> > cumbersome:
> > - developers need to create RBs and attach diff to JIRA
> > - committers need to review RBs, dowload diff and apply, then push.
> >
> > It would be much lighter if we take the pull request only approach, as
> > Kafka already converted to:
> > - for the developers, the only thing needed is to open a pull request.
> > - for committers, review and apply patch is from the same PR and merge
> can
> > be done directly on remote git repo.
> >
> > Of course, there might be some hookup scripts that we will need to link
> > JIRA w/ pull request in github, which Kafka already does. Any comments
> and
> > feedbacks are welcome!
> >
> > Thanks!
> >
> > -Yi
> >
>

Re: [DISCUSS] Moving to github/pull-request for code review and check-in

Posted by Boris Shkolnik <bo...@gmail.com>.
+1 for pull requests.

On Thu, Feb 18, 2016 at 3:53 PM, Yi Pan <ni...@gmail.com> wrote:

> Hi, all,
>
> I want to start the discussion on our code review/commit process.
>
> I felt that our code review and check-in process is a little bit
> cumbersome:
> - developers need to create RBs and attach diff to JIRA
> - committers need to review RBs, dowload diff and apply, then push.
>
> It would be much lighter if we take the pull request only approach, as
> Kafka already converted to:
> - for the developers, the only thing needed is to open a pull request.
> - for committers, review and apply patch is from the same PR and merge can
> be done directly on remote git repo.
>
> Of course, there might be some hookup scripts that we will need to link
> JIRA w/ pull request in github, which Kafka already does. Any comments and
> feedbacks are welcome!
>
> Thanks!
>
> -Yi
>