[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

2016-04-10 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1484#issuecomment-208088632 @rafaelweingartner Done. the Jira is CLOUDSTACK-9343 if you can take a look. Ty --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

2016-04-10 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1484#issuecomment-208087293 @rafaelweingartner I will open a Jira, the way that I found the problem was described in an answer to Daan Hoogland in the same PR. Ty --- If your

[GitHub] cloudstack pull request: Remove unused params from NetworkHelperIm...

2016-04-10 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1484#issuecomment-208086436 Hi @GabrielBrascher . I think this params are removed in this PR https://github.com/apache/cloudstack/pull/1447 =) Ty. --- If your project

[GitHub] cloudstack pull request: Removed unused methods from EventBus inte...

2016-04-08 Thread pedro-martins
Github user pedro-martins closed the pull request at: https://github.com/apache/cloudstack/pull/1260 --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9298: Improve performance of r...

2016-03-30 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1425#issuecomment-203654592 @nvazquez, Nice changes, great job refactoring your implementation. I found no more troubles in this PR, then LGTM. Ty. --- If your project is set

[GitHub] cloudstack pull request: CLOUDSTACK-9322: Support for Internal LB ...

2016-03-30 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1452#issuecomment-203581569 @nlivens great! And sorry for the delay =) I saw that you used the String.format as I suggested, but and about the using of CollectionUtils.isEmpty

[GitHub] cloudstack pull request: CLOUDSTACK-9319: Use timeout when applyin...

2016-03-27 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1451#discussion_r57537243 --- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java --- @@ -180,7 +179,7 @@ private Answer applyConfig

[GitHub] cloudstack pull request: CLOUDSTACK-9322: Support for Internal LB ...

2016-03-27 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1452#issuecomment-202163598 Hi @prashanthvarma. How about to use String.format() to create the strings in the loggers? The use of String format will turn the strings in the logs

[GitHub] cloudstack pull request: Removed unused parameters and variable fr...

2016-03-21 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1447#issuecomment-199337285 Hi @DaanHoogland. I did a maven install (this executes the unit tests). Also, the PR has passed in the Jenkins and the CI tests. However, I did

[GitHub] cloudstack pull request: Removed unused parameters and variable fr...

2016-03-19 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/1447 Removed unused parameters and variable from NetworkHelper hierarchy - Was removed the unused params User & Account in the com.cloud.network.router.NetworkHelper.startVirtualRo

[GitHub] cloudstack pull request: removed unused HypervDummyResourceBase cl...

2016-03-11 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1437#issuecomment-195364975 Hi @eriweb, thanks for reviewing this PR. I did a maven install in the ACS project and it built and ran all tests cases with no errors. But I didn't

[GitHub] cloudstack pull request: removed unused HypervDummyResourceBase cl...

2016-03-11 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1437#issuecomment-195361064 @rafaelweingartner ty for your review. Changed the log message. Ty. --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: removed unused HypervDummyResourceBase cl...

2016-03-10 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/1437 removed unused HypervDummyResourceBase class Was removed the class com.cloud.hypervisor.hyperv.resource.HypervDummyResourceBase - This class do not have the annotation

[GitHub] cloudstack pull request: Change variable "ROOK_DISK_CONTROLLER" to...

2016-03-10 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1434#issuecomment-194957493 Simple change in variable name, 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

[GitHub] cloudstack pull request: CLOUDSTACK-9298: Improve performance of r...

2016-03-04 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1425#issuecomment-192286165 @nvazquez. Could you squash all of yours commits into one? It's hard to compare the original code with your current modifications. If you do not know

[GitHub] cloudstack pull request: CLOUDSTACK-9298: Improve performance of r...

2016-03-02 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1425#discussion_r54797480 --- Diff: server/test/com/cloud/api/query/dao/TemplateJoinDaoImplTest.java --- @@ -0,0 +1,56 @@ +package com.cloud.api.query.dao

[GitHub] cloudstack pull request: CLOUDSTACK-9298: Improve performance of r...

2016-03-01 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1425#issuecomment-190874260 @nvazquez I mean unit tests for each new method. I think that use the Builder to get an instance with all variables initialized is a good

[GitHub] cloudstack pull request: [CLOUDSTACK-8973] Fix create template fro...

2016-03-01 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1424#discussion_r54595010 --- Diff: server/src/com/cloud/template/TemplateManagerImpl.java --- @@ -275,6 +277,8 @@ private StorageCacheManager cacheMgr

[GitHub] cloudstack pull request: CLOUDSTACK-9298: Improve performance of r...

2016-03-01 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1425#issuecomment-190791313 @nvazquez, nice changes. So, I guess that a test cases for these new methods will be important to detect future changes in these methods behavior

[GitHub] cloudstack pull request: CLOUDSTACK-9182: Some running VMs turned ...

2016-02-21 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1252#discussion_r53579947 --- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java --- @@ -1472,6 +1473,12 @@ private void advanceStop(final

[GitHub] cloudstack pull request: CLOUDSTACK-8829 : Consecutive cold migrat...

2016-02-21 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/797#discussion_r53579417 --- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java --- @@ -1776,19 +1773,26 @@ private void orchestrateStorageMigration

[GitHub] cloudstack pull request: CLOUDSTACK-8856 Primary Storage Used(type...

2016-02-05 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/865#discussion_r52029971 --- Diff: server/src/com/cloud/server/ManagementServerImpl.java --- @@ -2488,6 +2479,23 @@ public int compare(final SummedCapacity arg0, final

[GitHub] cloudstack pull request: CLOUDSTACK-8894: Restrict vGPU enabled VM...

2016-02-05 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/868#discussion_r52022212 --- Diff: server/src/com/cloud/vm/UserVmManagerImpl.java --- @@ -1535,6 +1535,19 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long

[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...

2016-01-31 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/556#discussion_r51372149 --- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java --- @@ -1765,7 +1767,18 @@ protected StartAnswer

[GitHub] cloudstack pull request: CLOUDSTACK-8931: Fail to deploy VM instan...

2016-01-31 Thread pedro-martins
Github user pedro-martins commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/907#discussion_r51360120 --- Diff: server/src/com/cloud/network/IpAddressManagerImpl.java --- @@ -680,10 +681,14 @@ public IPAddressVO doInTransaction(TransactionStatus

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-25 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-174539020 Yes, your import is correct, I've done a mistake, the collections4 (new collection version) isn't used by ACS, sry for that ^_^' --- If your project is set

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-25 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-174571489 There isn't a test for this class if you want a suggestion, create a new folder named test in cloud-controller-secondary-storage project and create a new

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-23 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-174210179 Could you replace the logic in your 'if' at lines 673 and 523 "(networks == null || networks.size() == 0)" to CollectionUtils.isEmpty(netw

[GitHub] cloudstack pull request: CLOUDSTACK-9100: ISO.CREATE/TEMPLATE.CREA...

2016-01-23 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1157#issuecomment-174213096 @ SudharmaJain why are you passing null to the method that you created (“createTemplateAsyncCallBack”) at line 503? That will cause a null pointer

[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...

2016-01-15 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1246#issuecomment-172029080 Hi =) What is the difference between network.getCidr() and network.getNetworkCidr() ? And, there is a possibility to you create a method with a simple

[GitHub] cloudstack pull request: CLOUDSTACK-8959: Option to attach the con...

2016-01-15 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/938#issuecomment-172103324 Hi. There is no problem to set vmData to null? and what about replace this short if to a method with documentation and a little test. And if you

[GitHub] cloudstack pull request: CLOUDSTACK-9180: Optimize concurrent VM d...

2016-01-11 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1251#issuecomment-170636370 A javadoc is a notation above the method declaration that describes what the method do. javadoc with examples :) http://www.oracle.com/technetwork

[GitHub] cloudstack pull request: CLOUDSTACK-9180: Optimize concurrent VM d...

2016-01-10 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/1251#issuecomment-170406819 nice =) Can you create a java doc to explain your 'isRouterDeployed()' (even the name explaining itself)? If you can create a test case for only

[GitHub] cloudstack pull request: Removed methods from EventBus interface.

2015-12-19 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/1260 Removed methods from EventBus interface. Was removed the methods . org.apache.cloudstack.framework.events.EventBus.subscribe(EventTopic, EventSubscriber

[GitHub] cloudstack pull request: Create test cases to getPatchFilePath met...

2015-11-23 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/944#issuecomment-158921322 Thanks for your review Daan. The purpose of this test is to check if an exception is thrown. The point is that, we have several class that we need

[GitHub] cloudstack pull request: Create test cases to getPatchFilePath met...

2015-11-23 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/944#issuecomment-158929670 Squash done. Thx guys --- 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

[GitHub] cloudstack pull request: Removed the PlannerBase class because it ...

2015-11-23 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/1108 Removed the PlannerBase class because it is does not bring contribution to ACS' code. Removed the PlannerBase class because it is does not bring contribution to ACS' code. We

[GitHub] cloudstack pull request: Create test cases to getPatchFilePath met...

2015-11-21 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/944#issuecomment-158667466 merged with master top version, someone can check this PR? =) thx guys. --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: Create test cases to getPatchFilePath met...

2015-10-17 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/944 Create test cases to getPatchFilePath method and class names changed In this commit we created tests cases for the respective classes in package â

[GitHub] cloudstack pull request: Removed the empty class 'com.cloud.deploy...

2015-09-21 Thread pedro-martins
GitHub user pedro-martins reopened a pull request: https://github.com/apache/cloudstack/pull/856 Removed the empty class 'com.cloud.deploy.PlannerBase' Removed the PlannerBase class because it is does not bring contribution to ACS' code. We changed

[GitHub] cloudstack pull request: Removed the empty class 'com.cloud.deploy...

2015-09-21 Thread pedro-martins
Github user pedro-martins closed the pull request at: https://github.com/apache/cloudstack/pull/856 --- 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

[GitHub] cloudstack pull request: Changed variable s_logger to non-static a...

2015-08-21 Thread pedro-martins
GitHub user pedro-martins reopened a pull request: https://github.com/apache/cloudstack/pull/714 Changed variable s_logger to non-static and fixed its name in “com.cloud.utils.component.ComponentLifecycleBase” and its subclasses Hi guys, We have noticed that every single

[GitHub] cloudstack pull request: Changed variable s_logger to non-static a...

2015-08-19 Thread pedro-martins
Github user pedro-martins closed the pull request at: https://github.com/apache/cloudstack/pull/714 --- 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

[GitHub] cloudstack pull request: Changed variable s_logger to non-static a...

2015-08-19 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/714#issuecomment-132654938 @DaanHoogland I agree with your considerations. My first commit was not performed properly. Sadly, the eclipse ended up formatting classes I touched

[GitHub] cloudstack pull request: Changed variable s_logger to non-static a...

2015-08-19 Thread pedro-martins
Github user pedro-martins commented on the pull request: https://github.com/apache/cloudstack/pull/714#issuecomment-132698414 Got your comments, sadly we have not analyzed which classes are singleton or not. We have opened a jira ticket to that: https://issues.apache.org/jira

[GitHub] cloudstack pull request: Changed variable s_logger to non-static a...

2015-08-18 Thread pedro-martins
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/714 Changed variable s_logger to non-static and fixed its name in “com.cloud.utils.component.ComponentLifecycleBase” and its subclasses Hi guys, We have noticed that every single