[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1254 --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206936718 Thank you guys. I will merge this... --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread abhinandanprateek
Github user abhinandanprateek commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206815106 @swill @bhaisaab @DaanHoogland I have done manual testing for the fix as expected. This can be merged now as there are enough LGTMs. --- If your project

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206773236 LGTM based on test/code, @DaanHoogland Abhi ( @agneya2001 ) can explain in much detail; but this fixes a NPE case and uses RoleType comparison that id comparison

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206755942 @swill @bhaisaab we should not accept an LTGM without explanation/justification --- If your project is set up for it, you can reply to this email and have

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206755664 LGTM, @swill based on the code --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-07 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206755482 LGTM, @swill --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-06 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206684033 We have one LGTM and it is passing CI, is this ready to merge then? --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-06 Thread abhinandanprateek
Github user abhinandanprateek commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1254#discussion_r58807499 --- Diff: plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java --- @@ -59,7 +59,7 @@ public QuotaSummaryCmd() {

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-06 Thread swill
Github user swill commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1254#discussion_r58776967 --- Diff: plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java --- @@ -59,7 +59,7 @@ public QuotaSummaryCmd() {

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-06 Thread bvbharatk
Github user bvbharatk commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-206542659 ### ACS CI BVT Run **Sumarry:** Build Number 159 Hypervisor xenserver NetworkType Advanced Passed=105 Failed=0 Skipped=4

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-04-03 Thread bvbharatk
Github user bvbharatk commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-205035894 ### ACS CI BVT Run **Sumarry:** Build Number 158 Hypervisor xenserver NetworkType Advanced Passed=106 Failed=2 Skipped=4

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-03-06 Thread alexandrelimassantana
Github user alexandrelimassantana commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1254#discussion_r55169349 --- Diff: framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java --- @@ -358,10 +358,11 @@ public QuotaUsageVO

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-01-27 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-175562144 @agneya2001 okay, we need one more LGTM on this. I'll picked your marvin fix from #1240 , can you also share test results if any? --- If your project is set up

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-01-26 Thread agneya2001
Github user agneya2001 commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-175372328 @cristofolini it is not that involved a code and not something that i can reuse. But yes i will take a holistic look at the code and refactor when I get

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-01-23 Thread cristofolini
Github user cristofolini commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-174217394 @agneya2001 Could you please extract the code at lines 126-135 to a separate method? It would make that bit of code much easier to read. :) --- If your

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2016-01-10 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-170456162 @remibergsma let's test and merge this before the freeze. thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2015-12-23 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/1254#issuecomment-167041889 LGTM --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2015-12-17 Thread resmo
Github user resmo commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1254#discussion_r47931786 --- Diff: plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java --- @@ -59,7 +59,7 @@ public QuotaSummaryCmd() {

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2015-12-17 Thread agneya2001
Github user agneya2001 commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1254#discussion_r47990946 --- Diff: plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java --- @@ -59,7 +59,7 @@ public QuotaSummaryCmd() {

[GitHub] cloudstack pull request: CLOUDSTACK-9174: A deleted account result...

2015-12-16 Thread agneya2001
GitHub user agneya2001 opened a pull request: https://github.com/apache/cloudstack/pull/1254 CLOUDSTACK-9174: A deleted account results in NPE When an account is deleted from cloudstack for which quota is still being calculated and if the quota reaches minimum threshold then