Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/845
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-142570859
Okay... we got 2 LGTM with proper testing/reporting. Thanks guys for taking
your time to look into it.
ping @borisroman @remibergsma @karuturi @wido
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-142520701
I am not merging this as I see a -1 from @wilderrodrigues
@wilderrodrigues, unittests are added and steps to test are also available.
Can you review again?
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-142520584
Tested this on a xenserver 6.5 setup. delete host/storage
pool/cluster/pod/network/zone are all successful.
:+1:
---
If your project is set up for it,
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-140983817
@borisroman Please ping me if this PR is ready to be tested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user borisroman commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/845#discussion_r39714178
--- Diff: engine/schema/src/com/cloud/network/dao/IPAddressDaoImpl.java ---
@@ -282,6 +289,14 @@ public IPAddressVO findByVmIp(String vmIp) {
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141022408
@remibergsma @wilderrodrigues @karuturi Please review!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141057501
@borisroman there are no (unit) tests. can you add or describe or at least
how to prove this fix?
---
If your project is set up for it, you can reply to this
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141059956
@DaanHoogland That was also one of my questions on Slack. There are no Unit
tests regarding Dao classes in /cloud-engine-schema/test/. So how do I test
these?
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141061213
@borisroman so please add a description of your manual test scheme in the
description area of this PR
( and help us think of a better dao implementation ;)
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141090948
:-1: until we get steps to test it.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email and have your
reply
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141124718
@borisroman Just successfully deleted a POD and a ZONE.
![screen shot 2015-09-17 at 17 02
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141086717
Hi @borisroman I deployed a DC and then added an extra POD. Then without
creating even a cluster, tried to delete it. It failed saying:
```
The pod
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141175510
@wilderrodrigues The steps to test this are simply creating a POD, then
deleting it. It fails in current master and succeeds in this PR. Same with a
zone. If
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/845#issuecomment-141170729
For the record, I tested against fa5f388fe91bc2fd477b1f6752c2a31ac69d5051.
---
If your project is set up for it, you can reply to this email and have your
reply
GitHub user borisroman opened a pull request:
https://github.com/apache/cloudstack/pull/845
[4.6][BLOCKER]CLOUDSTACK-8763: Resolved POD/ZONE deletion failure.
Instead of having both checkIfPodIsDeletable() and checkIfZoneIsDeletable
have there own SQL query, I've refactored them so
16 matches
Mail list logo
