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 user pedro-martins opened a pull request:
https://github.com/apache/cloudstack/pull/1261
Removed unused variables from class NetworkStateListener
Removed unused variables from class NetworkStateListener
We removed the following variables from
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 cas
Github user pedro-martins commented on the pull request:
https://github.com/apache/cloudstack/pull/1280#issuecomment-170409736
Nice :)
Can you extract the code to a method with a javadoc explaining the code?
If you can do a testcase for the method too, it will be apreciated
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 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 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 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.is
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 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
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
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 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 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 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 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 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 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 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 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 to
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 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.
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
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, or if the
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,
and that
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 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 user pedro-martins opened 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.
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, or if the
Github user pedro-martins commented on the pull request:
https://github.com/apache/cloudstack/pull/856#issuecomment-142042882
Reopening PR that was closed by mistake
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
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.
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 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 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 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 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
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 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 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 and
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&#
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 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 not
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 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 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() in
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 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, or if the
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 is
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 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
50 matches
Mail list logo
