You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Roshan Naik <ro...@hortonworks.com> on 2017/07/23 22:40:15 UTC
Request for holding off commits to storm-client module
Storm Devs,
My PR for STORM-2306 (messaging subsystem redesign) is almost ready. Not surprisingly, this PR brings extensive modifications in storm-client module and touches lightly on some others.
Just finished manually rebasing my local changes (to ~90 files) from an old version of master on to the latest master this Fri… but shortly after I was done, more commits came into storm-client and need some manual reconciling .. so it’s a bit difficult to keep up with.
So, would like to request committers and contributors looking to make changes in this critical module to either wait for this PR to go through or work with me to see if we can work something out if its critical.
Changes related to core areas like Disruptor, Metrics, Credential updates, Worker, Executor, Task, Bolt/Spout output collectors/executors, ACKing etc are areas with very high likelihood of merge conflicts.
If there are better alternative ideas happy to consider.
Thanks for understanding,
-roshan
Re: Request for holding off commits to storm-client module
Posted by Jungtaek Lim <ka...@gmail.com>.
Now pull request for STORM-2306 is up, and it looks like requiring several
weeks (months?) to be verified.
IMHO the effort of rebasing is unavoidable (though we could put effort to
minimize), especially pull requests were up already. Unfortunately, that's
about who takes the load, and I don't want to ask a favor of rebasing to
authors waiting for weeks and even some months to get their PR merged in.
So at least for earlier pull requests before the patch, I don't find a
reason to block putting them into storm-client module right now. Opinions?
- Jungtaek Lim (HeartSaVioR)
2017년 7월 24일 (월) 오후 4:30, Roshan Naik <ro...@hortonworks.com>님이 작성:
> I am fully ok with rebasing. I don’t expect development on Storm to cease
> while the PR gets reviewed. Its only changes to storm-client are likely to
> require to be done differently or not needed at all for the new system
> (like some fixes that went recently into Disruptor). So only for changes to
> storm-client module I am asking to see if the devs can work with me if its
> critical to get it in right away… only to avoid duplication of efforts or
> redundant efforts.
>
> Will have a PR out within the next 24 hours.
>
> -roshan
>
>
> On 7/23/17, 7:57 PM, "Jungtaek Lim" <ka...@gmail.com> wrote:
>
> Hi Roshan,
>
> I guess rebasing could be necessary even while in review phase, so
> personally I'd rather see the PR first even it has merge conflict
> between
> origin master branch, and have review process with changes in PR, and
> address once review process is done.
>
> If you want to hold off commits just before you submit a pull request,
> and
> you expect to submit it in several days, I'm also OK to wait for that.
> I
> just would like to ensure that holding doesn't stay longer.
>
> Thanks,
> Jungtaek Lim (HeartSaVioR)
>
> 2017년 7월 24일 (월) 오전 7:40, Roshan Naik <ro...@hortonworks.com>님이 작성:
>
> > Storm Devs,
> > My PR for STORM-2306 (messaging subsystem redesign) is almost
> ready. Not
> > surprisingly, this PR brings extensive modifications in storm-client
> module
> > and touches lightly on some others.
> >
> > Just finished manually rebasing my local changes (to ~90 files) from
> an
> > old version of master on to the latest master this Fri… but shortly
> after
> > I was done, more commits came into storm-client and need some manual
> > reconciling .. so it’s a bit difficult to keep up with.
> >
> > So, would like to request committers and contributors looking to make
> > changes in this critical module to either wait for this PR to go
> through or
> > work with me to see if we can work something out if its critical.
> >
> > Changes related to core areas like Disruptor, Metrics, Credential
> updates,
> > Worker, Executor, Task, Bolt/Spout output collectors/executors,
> ACKing etc
> > are areas with very high likelihood of merge conflicts.
> >
> > If there are better alternative ideas happy to consider.
> > Thanks for understanding,
> >
> > -roshan
> >
>
>
>
Re: Request for holding off commits to storm-client module
Posted by Roshan Naik <ro...@hortonworks.com>.
I am fully ok with rebasing. I don’t expect development on Storm to cease while the PR gets reviewed. Its only changes to storm-client are likely to require to be done differently or not needed at all for the new system (like some fixes that went recently into Disruptor). So only for changes to storm-client module I am asking to see if the devs can work with me if its critical to get it in right away… only to avoid duplication of efforts or redundant efforts.
Will have a PR out within the next 24 hours.
-roshan
On 7/23/17, 7:57 PM, "Jungtaek Lim" <ka...@gmail.com> wrote:
Hi Roshan,
I guess rebasing could be necessary even while in review phase, so
personally I'd rather see the PR first even it has merge conflict between
origin master branch, and have review process with changes in PR, and
address once review process is done.
If you want to hold off commits just before you submit a pull request, and
you expect to submit it in several days, I'm also OK to wait for that. I
just would like to ensure that holding doesn't stay longer.
Thanks,
Jungtaek Lim (HeartSaVioR)
2017년 7월 24일 (월) 오전 7:40, Roshan Naik <ro...@hortonworks.com>님이 작성:
> Storm Devs,
> My PR for STORM-2306 (messaging subsystem redesign) is almost ready. Not
> surprisingly, this PR brings extensive modifications in storm-client module
> and touches lightly on some others.
>
> Just finished manually rebasing my local changes (to ~90 files) from an
> old version of master on to the latest master this Fri… but shortly after
> I was done, more commits came into storm-client and need some manual
> reconciling .. so it’s a bit difficult to keep up with.
>
> So, would like to request committers and contributors looking to make
> changes in this critical module to either wait for this PR to go through or
> work with me to see if we can work something out if its critical.
>
> Changes related to core areas like Disruptor, Metrics, Credential updates,
> Worker, Executor, Task, Bolt/Spout output collectors/executors, ACKing etc
> are areas with very high likelihood of merge conflicts.
>
> If there are better alternative ideas happy to consider.
> Thanks for understanding,
>
> -roshan
>
Re: Request for holding off commits to storm-client module
Posted by Jungtaek Lim <ka...@gmail.com>.
Hi Roshan,
I guess rebasing could be necessary even while in review phase, so
personally I'd rather see the PR first even it has merge conflict between
origin master branch, and have review process with changes in PR, and
address once review process is done.
If you want to hold off commits just before you submit a pull request, and
you expect to submit it in several days, I'm also OK to wait for that. I
just would like to ensure that holding doesn't stay longer.
Thanks,
Jungtaek Lim (HeartSaVioR)
2017년 7월 24일 (월) 오전 7:40, Roshan Naik <ro...@hortonworks.com>님이 작성:
> Storm Devs,
> My PR for STORM-2306 (messaging subsystem redesign) is almost ready. Not
> surprisingly, this PR brings extensive modifications in storm-client module
> and touches lightly on some others.
>
> Just finished manually rebasing my local changes (to ~90 files) from an
> old version of master on to the latest master this Fri… but shortly after
> I was done, more commits came into storm-client and need some manual
> reconciling .. so it’s a bit difficult to keep up with.
>
> So, would like to request committers and contributors looking to make
> changes in this critical module to either wait for this PR to go through or
> work with me to see if we can work something out if its critical.
>
> Changes related to core areas like Disruptor, Metrics, Credential updates,
> Worker, Executor, Task, Bolt/Spout output collectors/executors, ACKing etc
> are areas with very high likelihood of merge conflicts.
>
> If there are better alternative ideas happy to consider.
> Thanks for understanding,
>
> -roshan
>