You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by GabrielBrascher <gi...@git.apache.org> on 2016/03/20 09:47:51 UTC
[GitHub] cloudstack pull request: Removed unused Classes
GitHub user GabrielBrascher opened a pull request:
https://github.com/apache/cloudstack/pull/1448
Removed unused Classes
Remove unused Classes (classes with no references:
- com.cloud.agent.api.CheckStateAnswer
- com.cloud.agent.api.StartupVMMAgentCommand
- com.cloud.agent.api.routing.UserDataCommand
- remove from description at
com.cloud.configuration.Config.ExecuteInSequenceNetworkElementCommands
enum
- com.cloud.agent.api.storage.UpgradeDiskCommand
- com.cloud.agent.api.storage.CreatePrivateTemplateCommand
- com.cloud.agent.api.storage.DestroyAnswer
- Note: there is just a line of `//FIXME: Should have an DestroyAnswer` at
com.cloud.storage.resource.StoragePoolResource
- com.cloud.agent.api.storage.UpgradeDiskAnswer
- com.cloud.agent.api.storage.ManageVolumeAvailabilityAnswer
- com.cloud.agent.api.storage.ManageVolumeAvailabilityCommand
- com.cloud.exception.UsageServerException
- com.cloud.info.SecStorageVmLoadInfo
- com.cloud.serializer.SerializerHelper
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/GabrielBrascher/cloudstack brascher-removeUnusedClasses
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1448.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1448
----
commit ce405b0426562d1758d33fc4f09bd3ae938adcee
Author: gabrascher <ga...@hotmail.com>
Date: 2016-03-20T08:44:30Z
Remove unused Classes (classes with no references; not in Spring
execution flow nor instantiated with "new"):
- com.cloud.agent.api.CheckStateAnswer
- com.cloud.agent.api.StartupVMMAgentCommand
- com.cloud.agent.api.routing.UserDataCommand
- remove from description at
com.cloud.configuration.Config.ExecuteInSequenceNetworkElementCommands
enum
- com.cloud.agent.api.storage.UpgradeDiskCommand
- com.cloud.agent.api.storage.CreatePrivateTemplateCommand
- com.cloud.agent.api.storage.DestroyAnswer
- Note: "FIXME: Should have an DestroyAnswer" at
com.cloud.storage.resource.StoragePoolResource
- com.cloud.agent.api.storage.UpgradeDiskAnswer
- com.cloud.agent.api.storage.ManageVolumeAvailabilityAnswer
- com.cloud.agent.api.storage.ManageVolumeAvailabilityCommand
- com.cloud.exception.UsageServerException
- com.cloud.info.SecStorageVmLoadInfo
- com.cloud.serializer.SerializerHelper
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199509482
With these major stripping out of unused code, we need to be very careful that we do not introduce integration test issues. Can you please validate that the integration test output does not change based on this code being removed?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199522152
Thank you @GabrielBrascher. I am also getting my CI setup so I can get tests running against these PRs, so I should be able to start validating this stuff as well. Thanks for the continued effort. :)
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199275394
Ok, that is somewhat comforting, however still a large integration test suite should be run on 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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199272015
@DaanHoogland the lack of test in some classes indeed is a problem. However, these classes that I removed are not being used.
I used UCDetector (http://www.ucdetector.org/) as a plugin for Eclipse. With this tool, I discovered a lot of code without any reference (variables, methods and classes).
This pull request had the goal of removing some of these classes. To ensure that I wasn't missing anything I searched for any file that could reference some answer or API command. As I haven't found any way of these classes being used, I removed them.
Any of those API commands that I deleted are not using org.apache.cloudstack.api.APICommand (with `@APICommand`). Therefore, those commands are not exposed in the ACS API.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199521289
@DaanHoogland @swill thank you for the concern. I agree with the need for executing integration/functional tests to keep the code stable. Unfortunately, our test environment is not set yet and it may take a while. As soon as we have it operational I will run the tests.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1448: CLOUDSTACK-9315: Removed unused Classes
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the issue:
https://github.com/apache/cloudstack/pull/1448
### ACS CI BVT Run
**Sumarry:**
Build Number 155
Hypervisor xenserver
NetworkType Advanced
Passed=68
Failed=5
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_vpc_vpn.py
* ContextSuite context=TestRVPCSite2SiteVpn>:setup Failing since 14 runs
* ContextSuite context=TestVpcRemoteAccessVpn>:setup Failing since 14 runs
* ContextSuite context=TestVpcSite2SiteVpn>:setup Failing since 14 runs
* test_volumes.py
* test_06_download_detached_volume Failed
* test_vm_life_cycle.py
* test_10_attachAndDetach_iso Failed
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9315: Removed unused Classes
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1448#issuecomment-199209016
@GabrielBrascher removing unused code is good; less is more. These are API calls and related stuff however. A large test set should be run and I am surprised no test code would be touched by this change.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---