You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by cristofolini <gi...@git.apache.org> on 2015/11/21 21:44:39 UTC
[GitHub] cloudstack pull request: Removed unnecessary @Local annotations an...
GitHub user cristofolini opened a pull request:
https://github.com/apache/cloudstack/pull/1102
Removed unnecessary @Local annotations and their respective imports
Following @rafaelweingartner 's findings in PR #714 that many spring beans contained an @Local annotation, we've decided to remove said annotations and their imports from the ComponentLifecycleBase class and its subclasses seeking a reduction of a few hundred lines of useless code.
I had already opened a pull request for this (#853) but at some point my commit disappeared from the PR entirely, showing no new changes in code, which caused it to be merged automatically (with no changes).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-006
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1102.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 #1102
----
commit 1a64c247adbeb326f7ddbe6e4feaf3159e9e740d
Author: cirstofolini <lu...@gmail.com>
Date: 2015-11-21T20:31:11Z
Removed unnecessary @Local annotations and their respective imports from the ComponentLifecycleBase class and its subclasses.
----
---
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: Removed unnecessary @Local annotations an...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1102#issuecomment-158987752
@cristofolini github doesn't show JuniperSRXExternalFirewallElement. It has a syntax error. Did you compile this? and did you use -Dnoredist? that part would have probably revealed the error.
---
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: Removed unnecessary @Local annotations an...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1102#issuecomment-158896104
LGTM, based on a set of tests that I run on this branch. Screenshot is from an experimental Jenkins job (that runs the same tests I did manually until now).
These test may not cover your change, all they do is show you didn't break them. Someone else needs to review 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 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: Removed unnecessary @Local annotations an...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1102
---
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: Removed unnecessary @Local annotations an...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1102#issuecomment-159047122
@DaanHoogland,
@cristofolini did not run a mvn build witth "-Dnoredist". Sadly, we did not realize that there were some projects that were not built without that parameter.
Sorry for the trouble.
From now on, we will make sure to run a "mvn clean install -Dnoredist -P developer,systemvm" to make sure that everything is ok, before we create a PR.
---
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: Removed unnecessary @Local annotations an...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1102#issuecomment-158896493
LGTM, checked the code. It felt like skimming at times as 607 lines were removed ;) I don't think there is any problem though.
---
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.
---