You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Remi Bergsma <RB...@schubergphilis.com> on 2015/08/09 20:49:38 UTC

Anyone wants to take over these orphaned PRs?

Hi all,

These are 9 PRs sent by Likitha. If I understand correctly Likitha is no longer working with Citrix/ACS. The PRs seem mostly VMware related. Some have LGTM(s), most have comments about missing unit tests.

What do we want to do with them? There's probably not gonna be an update from Likitha, so waiting for that does not make sense. Does anyone wants to step in and finalize a PR? You can get the PR to your own branch and add some work on top of the existing commits and finally send it as a new PR.

I'll add a comment to each of them. If no one wants to take it over, I think we should close the PRs without merging. It's a pity, but I would rather not have long lists of orphaned PRs laying around. The less PRs are open, the better.

Any comments?

Regards,
Remi

Cloudstack 8612 [VMware] #562
https://github.com/apache/cloudstack/pull/562

CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm... #561
https://github.com/apache/cloudstack/pull/561

CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro... #556
https://github.com/apache/cloudstack/pull/556

CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions issue. #555
https://github.com/apache/cloudstack/pull/555

CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ... #554
https://github.com/apache/cloudstack/pull/554
==> This one has 2xLGTM, but also some remarks to add unit tests.

CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat... #548
https://github.com/apache/cloudstack/pull/548
==> 1x LGTM

CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ... #547
https://github.com/apache/cloudstack/pull/547
==> 1x LGTM

CLOUDSTACK-8599. CS reports failure for a successful migration. #544
https://github.com/apache/cloudstack/pull/544

CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin... #540
https://github.com/apache/cloudstack/pull/540



Re: Anyone wants to take over these orphaned PRs?

Posted by Remi Bergsma <RB...@schubergphilis.com>.
Hi Suresh,

Thanks for picking this up! Some PRs got feedback, it’d be great if that could be addressed. In a perfect world, changes are submitted with tests. I see many reviewers ask for them, so adding them definitely helps in getting LGTMs. 

Not 100% sure what you mean by “Can we take up the testcases for LGTM(s) PRs later and merge them”. You want to add the tests later? You can also take some more time and send it all in once. No hurries.

Do you want to keep the current PRs open for now or should we close them?

Regards, Remi

> On 10 Aug 2015, at 06:25, Suresh Kumar Anaparti <su...@citrix.com> wrote:
> 
> Hi Remi,
> 
> I'll work on these PRs and finalize them. Will send new PRs for them. As Likitha is not responding, Can we take up the testcases for LGTM(s) PRs later and merge them?  
> 
> Thanks,
> Suresh
> 
> -----Original Message-----
> From: Remi Bergsma [mailto:RBergsma@schubergphilis.com] 
> Sent: Monday, 10 August 2015 12:20 AM
> To: 'dev@cloudstack.apache.org'
> Subject: Anyone wants to take over these orphaned PRs?
> 
> Hi all,
> 
> These are 9 PRs sent by Likitha. If I understand correctly Likitha is no longer working with Citrix/ACS. The PRs seem mostly VMware related. Some have LGTM(s), most have comments about missing unit tests.
> 
> What do we want to do with them? There's probably not gonna be an update from Likitha, so waiting for that does not make sense. Does anyone wants to step in and finalize a PR? You can get the PR to your own branch and add some work on top of the existing commits and finally send it as a new PR.
> 
> I'll add a comment to each of them. If no one wants to take it over, I think we should close the PRs without merging. It's a pity, but I would rather not have long lists of orphaned PRs laying around. The less PRs are open, the better.
> 
> Any comments?
> 
> Regards,
> Remi
> 
> Cloudstack 8612 [VMware] #562
> https://github.com/apache/cloudstack/pull/562
> 
> CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm... #561
> https://github.com/apache/cloudstack/pull/561
> 
> CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro... #556
> https://github.com/apache/cloudstack/pull/556
> 
> CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions issue. #555
> https://github.com/apache/cloudstack/pull/555
> 
> CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ... #554
> https://github.com/apache/cloudstack/pull/554
> ==> This one has 2xLGTM, but also some remarks to add unit tests.
> 
> CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat... #548
> https://github.com/apache/cloudstack/pull/548
> ==> 1x LGTM
> 
> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ... #547
> https://github.com/apache/cloudstack/pull/547
> ==> 1x LGTM
> 
> CLOUDSTACK-8599. CS reports failure for a successful migration. #544
> https://github.com/apache/cloudstack/pull/544
> 
> CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin... #540
> https://github.com/apache/cloudstack/pull/540
> 
> 


RE: Anyone wants to take over these orphaned PRs?

Posted by Suresh Kumar Anaparti <su...@citrix.com>.
Hi Remi,

I'll work on these PRs and finalize them. Will send new PRs for them. As Likitha is not responding, Can we take up the testcases for LGTM(s) PRs later and merge them?  

Thanks,
Suresh

-----Original Message-----
From: Remi Bergsma [mailto:RBergsma@schubergphilis.com] 
Sent: Monday, 10 August 2015 12:20 AM
To: 'dev@cloudstack.apache.org'
Subject: Anyone wants to take over these orphaned PRs?

Hi all,

These are 9 PRs sent by Likitha. If I understand correctly Likitha is no longer working with Citrix/ACS. The PRs seem mostly VMware related. Some have LGTM(s), most have comments about missing unit tests.

What do we want to do with them? There's probably not gonna be an update from Likitha, so waiting for that does not make sense. Does anyone wants to step in and finalize a PR? You can get the PR to your own branch and add some work on top of the existing commits and finally send it as a new PR.

I'll add a comment to each of them. If no one wants to take it over, I think we should close the PRs without merging. It's a pity, but I would rather not have long lists of orphaned PRs laying around. The less PRs are open, the better.

Any comments?

Regards,
Remi

Cloudstack 8612 [VMware] #562
https://github.com/apache/cloudstack/pull/562

CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm... #561
https://github.com/apache/cloudstack/pull/561

CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro... #556
https://github.com/apache/cloudstack/pull/556

CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions issue. #555
https://github.com/apache/cloudstack/pull/555

CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ... #554
https://github.com/apache/cloudstack/pull/554
==> This one has 2xLGTM, but also some remarks to add unit tests.

CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat... #548
https://github.com/apache/cloudstack/pull/548
==> 1x LGTM

CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ... #547
https://github.com/apache/cloudstack/pull/547
==> 1x LGTM

CLOUDSTACK-8599. CS reports failure for a successful migration. #544
https://github.com/apache/cloudstack/pull/544

CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin... #540
https://github.com/apache/cloudstack/pull/540



Re: Anyone wants to take over these orphaned PRs?

Posted by Mike Tutkowski <mi...@solidfire.com>.
I went ahead and tested, then checked in PR 547:

https://github.com/apache/cloudstack/pull/547/files

I also resolved and closed the JIRA ticket for it:

https://issues.apache.org/jira/browse/CLOUDSTACK-8601

On Mon, Aug 10, 2015 at 11:43 AM, Mike Tutkowski <
mike.tutkowski@solidfire.com> wrote:

> I should be able to test it.
>
> Talk to you soon,
> Mike
>
> On Sun, Aug 9, 2015 at 10:40 PM, Remi Bergsma <RBergsma@schubergphilis.com
> > wrote:
>
>> Hi Mike,
>>
>> Thanks! It seems the testing / verification is still to do and that is
>> the work. Merging itself can be done with a one-liner.
>>
>> Are you able to verify the fix in your lab?
>>
>> Sent from my iPhone
>>
>> > On 10 Aug 2015, at 04:13, Mike Tutkowski <mi...@solidfire.com>
>> wrote:
>> >
>> > I could take this one:
>> >
>> > CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
>> > #547
>> > https://github.com/apache/cloudstack/pull/547
>> > ==> 1x LGTM
>> >
>> > I asked this in the PR discussion, but are we under the impression that
>> > this code has been tested well enough? If so, I can just merge it as the
>> > code itself LGTM.
>> >
>> > On Sun, Aug 9, 2015 at 12:49 PM, Remi Bergsma <
>> RBergsma@schubergphilis.com>
>> > wrote:
>> >
>> >> Hi all,
>> >>
>> >> These are 9 PRs sent by Likitha. If I understand correctly Likitha is
>> no
>> >> longer working with Citrix/ACS. The PRs seem mostly VMware related.
>> Some
>> >> have LGTM(s), most have comments about missing unit tests.
>> >>
>> >> What do we want to do with them? There's probably not gonna be an
>> update
>> >> from Likitha, so waiting for that does not make sense. Does anyone
>> wants to
>> >> step in and finalize a PR? You can get the PR to your own branch and
>> add
>> >> some work on top of the existing commits and finally send it as a new
>> PR.
>> >>
>> >> I'll add a comment to each of them. If no one wants to take it over, I
>> >> think we should close the PRs without merging. It's a pity, but I would
>> >> rather not have long lists of orphaned PRs laying around. The less PRs
>> are
>> >> open, the better.
>> >>
>> >> Any comments?
>> >>
>> >> Regards,
>> >> Remi
>> >>
>> >> Cloudstack 8612 [VMware] #562
>> >> https://github.com/apache/cloudstack/pull/562
>> >>
>> >> CLOUDSTACK-8611. CS waits indefinitely for
>> CheckS2SVpnConnectionsComm...
>> >> #561
>> >> https://github.com/apache/cloudstack/pull/561
>> >>
>> >> CLOUDSTACK-8609. [VMware] VM is not accessible after a migration
>> acro...
>> >> #556
>> >> https://github.com/apache/cloudstack/pull/556
>> >>
>> >> CLOUDSTACK-8608. [VMware] System VM's failed to start due to
>> permissions
>> >> issue. #555
>> >> https://github.com/apache/cloudstack/pull/555
>> >>
>> >> CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2
>> ...
>> >> #554
>> >> https://github.com/apache/cloudstack/pull/554
>> >> ==> This one has 2xLGTM, but also some remarks to add unit tests.
>> >>
>> >> CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain
>> dat...
>> >> #548
>> >> https://github.com/apache/cloudstack/pull/548
>> >> ==> 1x LGTM
>> >>
>> >> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added
>> ...
>> >> #547
>> >> https://github.com/apache/cloudstack/pull/547
>> >> ==> 1x LGTM
>> >>
>> >> CLOUDSTACK-8599. CS reports failure for a successful migration. #544
>> >> https://github.com/apache/cloudstack/pull/544
>> >>
>> >> CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves
>> behin...
>> >> #540
>> >> https://github.com/apache/cloudstack/pull/540
>> >
>> >
>> > --
>> > *Mike Tutkowski*
>> > *Senior CloudStack Developer, SolidFire Inc.*
>> > e: mike.tutkowski@solidfire.com
>> > o: 303.746.7302
>> > Advancing the way the world uses the cloud
>> > <http://solidfire.com/solution/overview/?video=play>*™*
>>
>
>
>
> --
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the cloud
> <http://solidfire.com/solution/overview/?video=play>*™*
>



-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*

Re: Anyone wants to take over these orphaned PRs?

Posted by Mike Tutkowski <mi...@solidfire.com>.
I should be able to test it.

Talk to you soon,
Mike

On Sun, Aug 9, 2015 at 10:40 PM, Remi Bergsma <RB...@schubergphilis.com>
wrote:

> Hi Mike,
>
> Thanks! It seems the testing / verification is still to do and that is the
> work. Merging itself can be done with a one-liner.
>
> Are you able to verify the fix in your lab?
>
> Sent from my iPhone
>
> > On 10 Aug 2015, at 04:13, Mike Tutkowski <mi...@solidfire.com>
> wrote:
> >
> > I could take this one:
> >
> > CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
> > #547
> > https://github.com/apache/cloudstack/pull/547
> > ==> 1x LGTM
> >
> > I asked this in the PR discussion, but are we under the impression that
> > this code has been tested well enough? If so, I can just merge it as the
> > code itself LGTM.
> >
> > On Sun, Aug 9, 2015 at 12:49 PM, Remi Bergsma <
> RBergsma@schubergphilis.com>
> > wrote:
> >
> >> Hi all,
> >>
> >> These are 9 PRs sent by Likitha. If I understand correctly Likitha is no
> >> longer working with Citrix/ACS. The PRs seem mostly VMware related. Some
> >> have LGTM(s), most have comments about missing unit tests.
> >>
> >> What do we want to do with them? There's probably not gonna be an update
> >> from Likitha, so waiting for that does not make sense. Does anyone
> wants to
> >> step in and finalize a PR? You can get the PR to your own branch and add
> >> some work on top of the existing commits and finally send it as a new
> PR.
> >>
> >> I'll add a comment to each of them. If no one wants to take it over, I
> >> think we should close the PRs without merging. It's a pity, but I would
> >> rather not have long lists of orphaned PRs laying around. The less PRs
> are
> >> open, the better.
> >>
> >> Any comments?
> >>
> >> Regards,
> >> Remi
> >>
> >> Cloudstack 8612 [VMware] #562
> >> https://github.com/apache/cloudstack/pull/562
> >>
> >> CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm...
> >> #561
> >> https://github.com/apache/cloudstack/pull/561
> >>
> >> CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro...
> >> #556
> >> https://github.com/apache/cloudstack/pull/556
> >>
> >> CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions
> >> issue. #555
> >> https://github.com/apache/cloudstack/pull/555
> >>
> >> CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ...
> >> #554
> >> https://github.com/apache/cloudstack/pull/554
> >> ==> This one has 2xLGTM, but also some remarks to add unit tests.
> >>
> >> CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat...
> >> #548
> >> https://github.com/apache/cloudstack/pull/548
> >> ==> 1x LGTM
> >>
> >> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
> >> #547
> >> https://github.com/apache/cloudstack/pull/547
> >> ==> 1x LGTM
> >>
> >> CLOUDSTACK-8599. CS reports failure for a successful migration. #544
> >> https://github.com/apache/cloudstack/pull/544
> >>
> >> CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin...
> >> #540
> >> https://github.com/apache/cloudstack/pull/540
> >
> >
> > --
> > *Mike Tutkowski*
> > *Senior CloudStack Developer, SolidFire Inc.*
> > e: mike.tutkowski@solidfire.com
> > o: 303.746.7302
> > Advancing the way the world uses the cloud
> > <http://solidfire.com/solution/overview/?video=play>*™*
>



-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*

Re: Anyone wants to take over these orphaned PRs?

Posted by Remi Bergsma <RB...@schubergphilis.com>.
Hi Mike,

Thanks! It seems the testing / verification is still to do and that is the work. Merging itself can be done with a one-liner. 

Are you able to verify the fix in your lab?

Sent from my iPhone

> On 10 Aug 2015, at 04:13, Mike Tutkowski <mi...@solidfire.com> wrote:
> 
> I could take this one:
> 
> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
> #547
> https://github.com/apache/cloudstack/pull/547
> ==> 1x LGTM
> 
> I asked this in the PR discussion, but are we under the impression that
> this code has been tested well enough? If so, I can just merge it as the
> code itself LGTM.
> 
> On Sun, Aug 9, 2015 at 12:49 PM, Remi Bergsma <RB...@schubergphilis.com>
> wrote:
> 
>> Hi all,
>> 
>> These are 9 PRs sent by Likitha. If I understand correctly Likitha is no
>> longer working with Citrix/ACS. The PRs seem mostly VMware related. Some
>> have LGTM(s), most have comments about missing unit tests.
>> 
>> What do we want to do with them? There's probably not gonna be an update
>> from Likitha, so waiting for that does not make sense. Does anyone wants to
>> step in and finalize a PR? You can get the PR to your own branch and add
>> some work on top of the existing commits and finally send it as a new PR.
>> 
>> I'll add a comment to each of them. If no one wants to take it over, I
>> think we should close the PRs without merging. It's a pity, but I would
>> rather not have long lists of orphaned PRs laying around. The less PRs are
>> open, the better.
>> 
>> Any comments?
>> 
>> Regards,
>> Remi
>> 
>> Cloudstack 8612 [VMware] #562
>> https://github.com/apache/cloudstack/pull/562
>> 
>> CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm...
>> #561
>> https://github.com/apache/cloudstack/pull/561
>> 
>> CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro...
>> #556
>> https://github.com/apache/cloudstack/pull/556
>> 
>> CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions
>> issue. #555
>> https://github.com/apache/cloudstack/pull/555
>> 
>> CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ...
>> #554
>> https://github.com/apache/cloudstack/pull/554
>> ==> This one has 2xLGTM, but also some remarks to add unit tests.
>> 
>> CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat...
>> #548
>> https://github.com/apache/cloudstack/pull/548
>> ==> 1x LGTM
>> 
>> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
>> #547
>> https://github.com/apache/cloudstack/pull/547
>> ==> 1x LGTM
>> 
>> CLOUDSTACK-8599. CS reports failure for a successful migration. #544
>> https://github.com/apache/cloudstack/pull/544
>> 
>> CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin...
>> #540
>> https://github.com/apache/cloudstack/pull/540
> 
> 
> -- 
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the cloud
> <http://solidfire.com/solution/overview/?video=play>*™*

Re: Anyone wants to take over these orphaned PRs?

Posted by Mike Tutkowski <mi...@solidfire.com>.
I could take this one:

CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
#547
https://github.com/apache/cloudstack/pull/547
==> 1x LGTM

I asked this in the PR discussion, but are we under the impression that
this code has been tested well enough? If so, I can just merge it as the
code itself LGTM.

On Sun, Aug 9, 2015 at 12:49 PM, Remi Bergsma <RB...@schubergphilis.com>
wrote:

> Hi all,
>
> These are 9 PRs sent by Likitha. If I understand correctly Likitha is no
> longer working with Citrix/ACS. The PRs seem mostly VMware related. Some
> have LGTM(s), most have comments about missing unit tests.
>
> What do we want to do with them? There's probably not gonna be an update
> from Likitha, so waiting for that does not make sense. Does anyone wants to
> step in and finalize a PR? You can get the PR to your own branch and add
> some work on top of the existing commits and finally send it as a new PR.
>
> I'll add a comment to each of them. If no one wants to take it over, I
> think we should close the PRs without merging. It's a pity, but I would
> rather not have long lists of orphaned PRs laying around. The less PRs are
> open, the better.
>
> Any comments?
>
> Regards,
> Remi
>
> Cloudstack 8612 [VMware] #562
> https://github.com/apache/cloudstack/pull/562
>
> CLOUDSTACK-8611. CS waits indefinitely for CheckS2SVpnConnectionsComm...
> #561
> https://github.com/apache/cloudstack/pull/561
>
> CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro...
> #556
> https://github.com/apache/cloudstack/pull/556
>
> CLOUDSTACK-8608. [VMware] System VM's failed to start due to permissions
> issue. #555
> https://github.com/apache/cloudstack/pull/555
>
> CLOUDSTACK-8610. Unable to attach 7th Disk to Windows Server 2012 R2 ...
> #554
> https://github.com/apache/cloudstack/pull/554
> ==> This one has 2xLGTM, but also some remarks to add unit tests.
>
> CLOUDSTACK-8602. MigrateVirtualMachineWithVolume leaves old chain dat...
> #548
> https://github.com/apache/cloudstack/pull/548
> ==> 1x LGTM
>
> CLOUDSTACK-8601. VMFS storage added as local storage can be re-added ...
> #547
> https://github.com/apache/cloudstack/pull/547
> ==> 1x LGTM
>
> CLOUDSTACK-8599. CS reports failure for a successful migration. #544
> https://github.com/apache/cloudstack/pull/544
>
> CLOUDSTACK-8415. SSVM shutdown during snapshot operation leaves behin...
> #540
> https://github.com/apache/cloudstack/pull/540
>
>
>


-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*