search

sbt / sbt-osgi

sbt plugin for creating OSGi bundles
Apache License 2.0
47 stars 43 forks source link

Use dependencies as JARs for force proper manifests (fixes #60) #64

Closed lefou closed 5 years ago

lefou commented 5 years ago

This fixes #60.

This PR contains the same fixes as PR #61, but without adding the flaky integration test. #61 was reverted because of these flaky tests.

lefou commented 5 years ago

Review by @johanandren

lefou commented 5 years ago

The Travis build failed, because oraclejdk10 is deprecated. But it succeeded for oraclejdk8.

atooni commented 5 years ago

This PR comes from our team building a scala/akka based integration framework at https://github.com/woq-blended/blended. We are using a snapshot version with the patch in our own build. It works perfectly well in our set up and all bundles we generate using the patched version work as expected.

From our perspective the PR is good to merge and works as expected.

This having said, we follow a slightly different approach in our setup than most other projects we have seen. On other projects we often see that a Scala class within the project folder is used to contribute the setting for a feature (i.e. OSGi) for ALL configured subprojects. As a result, an individual sub project usually consists of the basic setup + n contributions.

We have chosen to have 1 file / subproject and configure all the features for a subproject in one place - using helper classes to abstract away common settings where it makes sense. We do the same for OSGi and we do find that keeps our setup quite clean.

FYI: http://woq-blended.github.io/blended/BUILDING.html

lefou commented 5 years ago

I thought, the decision to merge this PR was already made (see #61), but later reverted because of the (known) flaky integration test, which I left out this time.

johanandren commented 5 years ago

We asked for a second opinion from the people who have contributed to the OSGi support in Akka, but I interpret this as good to go:

From our perspective the PR is good to merge and works as expected.

Thanks for the extra check @atooni

lefou commented 5 years ago

Can we get a new release, please?