-
Notifications
You must be signed in to change notification settings - Fork 605
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
Removed some Felix SCR annotations and replaced with OSGi annotations #3332
base: master
Are you sure you want to change the base?
Removed some Felix SCR annotations and replaced with OSGi annotations #3332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple public abstract classes should probably retain the SCR annotation variants in addition to the DS annotations. Because they don't have @Component
annotations it shouldn't result in duplicate or conflicting SCR descriptors for them.
import org.apache.sling.api.SlingHttpServletRequest; | ||
import org.osgi.framework.ServiceRegistration; | ||
import org.osgi.service.component.ComponentContext; | ||
import org.osgi.service.component.annotations.Deactivate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is a public abstract class, I recommend leaving the SCR annotation in place here, in addition to the DS annotation, so that consumers using Felix SCR tooling will still find the old annotation in the class file.
@@ -38,6 +38,8 @@ | |||
import org.osgi.framework.ServiceReference; | |||
import org.osgi.framework.ServiceRegistration; | |||
import org.osgi.service.component.ComponentContext; | |||
import org.osgi.service.component.annotations.Activate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is a public abstract class, I recommend leaving the SCR annotation in place here, in addition to the DS annotation, so that consumers using Felix SCR tooling will still find the old annotation in the class file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. As in:
@Activate
@org.apache.felix.scr.annotations.Activate
protected void activate() {
}
Any customers with components that extend from these classes without overriding the annotated method would still have to be using SCR annotations. (Using DS component inheritance is discouraged, but it is supported by default with maven-bundle-plugin when using SCR annotations).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn’t consider such edge cases. Too much maintenance overhead and easy to fix on consumer side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us use Custom Sling Servlet Property Type Annotations
Didnt do the components that require OCDs:
Remaining