Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

added support for annotation default values and replacement of sysprops #3

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

Conversation

hsel
Copy link

@hsel hsel commented Oct 17, 2017

No description provided.

@magnet
Copy link
Contributor

magnet commented Oct 17, 2017

Hi @hsel,

Thanks for your pull-request!

Back when I first built the plugin, bnd had the annotationDefault() methods but they were not implemented, so I gave up on that feature. It was 3.0.0 back then and a lot of work has been done since. With which version of bnd are you using that patch? I would like to test it before merging!

@magnet magnet self-assigned this Oct 17, 2017
@hsel
Copy link
Author

hsel commented Oct 18, 2017

Hi Simon,

I use maven-bundle-plugin 3.0.1 and 3.3.0 (using bnd 3.0.0 respectively 3.3.0). Works with both versions.

Please let me know if you have further questions.

Holger

@magnet
Copy link
Contributor

magnet commented Oct 18, 2017

@hsel OK thanks, I will test with bnd 3.4.0 and 3.5.0 and if it works then merge the branch. Since bndlib had API changes I want to make sure it will work on recent versions first. I will have a look maybe next week or the one after that, and also cut a release if everything is ok.

@magnet
Copy link
Contributor

magnet commented Nov 13, 2017

Hi @hsel ,

So I gave a bit more thought to your proposal. Default method support is great but I'm not sure about expanding properties at build time.
In OSGi R7 the Configuration Admin spec update will allow us to have Configurers that can repace such properties at runtime, which seems like something we might want to do as well.

I thought that properties that we want expanded at build-time should be annotated as such with an explicit flag in the annotation (in this case, failing if no property that name can be found) or with a flag in the bnd configuration for the plugin.

What do you think? I have no problem making the changes but I have stuff on my TODO list before doing so.
I'd be happy to merge the default annotation support before!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants