Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAXCDI-196 - It does not work to intercept the events start and stop the bundles. #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lokot
Copy link

@Lokot Lokot commented Jul 7, 2015

PAXCDI-196: fix NPE in ServiceEventBridge, enabled BundleEventBridge and fire events to observers

@hwellmann
Copy link
Member

Please mind the indentation and formatting conventions before creating a pull request. It's hard to see your actual changes.

Pax CDI uses the formatter from https://github.com/ops4j/org.ops4j.pax.exam2/blob/master/assets/EclipseJavaFormatter.xml.

@Lokot Lokot force-pushed the PAXCDI-196 branch 2 times, most recently from 76c05d3 to 40c353e Compare July 8, 2015 06:53
@Lokot
Copy link
Author

Lokot commented Jul 8, 2015

I changed the code formatting. Please look.

@kwesterfeld
Copy link

Please review and accept this PR. I myself would like to use pax-cdi but there is no way to hook bundle stopped. Unfortunately, the lifecycle handling for @PreDestroy seems to only work when you shutdown the container. In my case, I was using Karaf. My expectation was that @PreDestroy would be called on bundle stop, which is not the case. Nor does it get called on bundle update.

This patch would give me another route to hook shutdown logic, which I require.

@astefanutti
Copy link
Contributor

Can that PR be reviewed and eventually merged?

@hwellmann
Copy link
Member

We don't merge PRs without tests.

This topic is known to fail a test, see comments in https://ops4j1.jira.com/browse/PAXCDI-6 for details.

If someone can make bundle events work in a robust manner and provide reasonable test coverage, I'm happy to merge the solution.

@astefanutti
Copy link
Contributor

@hwellmann thanks for the details. That's clearer what the status is.

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.

4 participants