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.

Sign up for GitHub

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

Jump to bottom

First annotation for ServiceTracker in org.eclipse.osgi #59

Merged
merged 3 commits into from
Nov 23, 2017
Merged

First annotation for ServiceTracker in org.eclipse.osgi #59

merged 3 commits into from
Nov 23, 2017

Conversation

triller-telekom
Copy link
Contributor

Signed-off-by: Stefan Triller stefan.triller@telekom.de

@triller-telekom
First annotation for ServiceTracker in org.eclipse.osgi
cb713a4 
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@triller-telekom
eeav marker
0b70c37 
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@vorburger vorburger self-requested a review November 14, 2017 15:14
vorburger
vorburger requested changes Nov 14, 2017
View reviewed changes
Copy link
Member

@vorburger vorburger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This could later even be handy for future null testing over in our opendaylight.org code as well (where we try not to depend on Equinox artifacts, just org.osgi)... but given that this is an EEA for org.osgi.util.tracker.ServiceTracker IMHO it should be eea-for-gav org.osgi:org.osgi.core and not be in directory libraries/org.eclipse.osgi but in like libraries/osgi-core and osgi-core-eea, instead of any org.eclipse, what do you think?

@triller-telekom
renamed artifact
d1c36f0 
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@triller-telekom
Copy link
Contributor Author

To be honest my naming was just a guess :) But since you wrote speak in code not words! in eclipse-archived/smarthome#4217 (comment) I thought I give it a try and come up with a first PR for an annotation that I stumbled across while preparing eclipse-archived/smarthome#4550

So I have changed the naming to what you suggested.

vorburger
vorburger approved these changes Nov 14, 2017
View reviewed changes
Copy link
Member

@vorburger vorburger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Let me give other folks here say 48h if any other feedback, else I'll merge this...

@vorburger vorburger merged commit ed75590 into lastnpe:master Nov 23, 2017
@triller-telekom triller-telekom deleted the osgiAnnotations branch November 24, 2017 08:16
@vorburger vorburger mentioned this pull request Nov 24, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vorburger vorburger vorburger approved these changes

@maggu2810 maggu2810 Awaiting requested review from maggu2810

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@triller-telekom @vorburger