Skip to content

Conversation

@cristofolini
Copy link
Contributor

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).

…rom the ComponentLifecycleBase class and its subclasses.
@remibergsma
Copy link
Contributor

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.

screen shot 2015-11-23 at 11 13 16

@DaanHoogland
Copy link
Contributor

LGTM, checked the code. It felt like skimming at times as 607 lines were removed ;) I don't think there is any problem though.

@asfgit asfgit merged commit 1a64c24 into apache:master Nov 23, 2015
asfgit pushed a commit that referenced this pull request Nov 23, 2015
…-006

Removed unnecessary @Local annotations and their respective importsFollowing @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).

* pr/1102:
  Removed unnecessary @Local annotations and their respective imports from the ComponentLifecycleBase class and its subclasses.

Signed-off-by: Remi Bergsma <[email protected]>
@DaanHoogland
Copy link
Contributor

@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.

I am busy creating a fix

@rafaelweingartner
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants