killbill / killbill-plugin-framework-ruby

Framework to write Kill Bill plugins in Ruby
http://killbill.io
8 stars 11 forks source link

re-master #41

Closed kares closed 9 years ago

kares commented 9 years ago

mostly minors (for now) ... see commits

pierre commented 9 years ago

:+1: I've started a kares-next branch!

kares commented 9 years ago

but this will require a change in the release.sh script

I see ... will look into that later. also wanted to add the 'unified' configuration (spec) related changes as I get there.

Isn't it a good practice to avoid relying on transitive dependencies (the plugins explicitly invoke Sinatra)?

Sinatra seems a bit "leaky" ... (while I do not completely understand how it's used) it feels like the plugin does not need to care about the FX ... the config.ru might become run KillBill::Application

... but back to the question RG/Bundler do a good job around deps, any issues with transitive ones?

pierre commented 9 years ago

(while I do not completely understand how it's used)

The overall idea is to allow plugins to expose (private) HTTP endpoints using the same container as Kill Bill. The main use case is for plugins to expose APIs that go beyond the standard Kill Bill plugin APIs (plugin specific business logic).

In practice, we proxy requests to /plugins from the JAX-RS layer to the OSGI layer and eventually to the Ruby plugins. From there, the request goes through Rack and finally Sinatra in the plugin itself.

(straightforward, right? :smile_cat:)

it feels like the plugin does not need to care about the FX ... the config.ru might become run KillBill::Application

What's FX?

... but back to the question RG/Bundler do a good job around deps, any issues with transitive ones?

No specific issue. In Java land, it is considered best practice to not rely on transitive dependencies to correctly control the versioning. I assumed it was the same with Bundler. If not, we could indeed probably further simplify the gemspec.

kares commented 9 years ago

In practice, we proxy requests to /plugins from the JAX-RS layer to the OSGI layer and eventually to the Ruby plugins. From there, the request goes through Rack and finally Sinatra in the plugin itself.

yes - yes ... just never seen it "in action" so far ... never mind - maybe some day :)

What's FX?

framework (btw. good that you brought that up :) - it's "leaking" into every plugin that sinatra is used as a backend, while it needs not e.g. if you simply alias KillBill::Application = Sinatra::Application

... but I take my notes back I forgot that there's specific Sinatra code in every plugin (assumed it only lives in killbill gem) such as https://github.com/killbill/killbill-stripe-plugin/blob/master/lib/stripe/application.rb ... thus that change makes no sense (will update to keep sinatra as an explicit dependency)

kares commented 9 years ago

de-ja-vu

but this will require a change in the release.sh script (https://github.com/killbill/killbill-plugin-framework-ruby/blob/master/generators/active_merchant/templates/release.sh.rb#L21).

actually it won't require a change there, only removed the VERSION file here (not for the plugin template) for plugins it's not that useful to have a VERSION constant since they're more of an "end" product not someone else's dependency ... the killbill gem seems to be released the standard ruby way (rake release) thus should be fine.

Isn't it a good practice to avoid relying on transitive dependencies (the plugins explicitly invoke Sinatra)?

removed the commit which deleted sinatra gem dependency on the plugin template

pierre commented 9 years ago

actually it won't require a change there, only removed the VERSION file here

:+1:

removed the commit which deleted sinatra gem dependency on the plugin template

:+1:

Thanks!