+1! Many thanks @demobox!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/79#issuecomment-246259756
@nacx Updated with some additional changes
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/79#issuecomment-246219889
Just two final comments, but LGTM!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/79#issuecomment-246199894
Do you think it makes sense to move the copied classes to their own package so
they are easy to reference (if that is needed or helps at some point)?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> @@ -51,8 +52,9 @@ public String evaluate(Object obj, String expression) {
> try {
>scriptEngine.put(getType(), obj);
>result = String.valueOf(scriptEngine.eval(expression));
> -} catch (Exception ex) {
> - //Ignore
> +} catch (Exception exception) {
> +
>private final ScriptEngine scriptEngine;
>
>/**
> * Constructor
> * @param engine
> */
> - public ScriptEngineShellTable(String engine) {
> -this.engine = engine;
> -this.scriptEngine = scriptEngineFactory.getEngineByName(engine);
> + public
@nacx Reformatted and added the ISE if the requested engine can't be loaded.
Please take a look to see there's anything that still needs to be changed!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> @@ -51,8 +52,9 @@ public String evaluate(Object obj, String expression) {
> try {
>scriptEngine.put(getType(), obj);
>result = String.valueOf(scriptEngine.eval(expression));
> -} catch (Exception ex) {
> - //Ignore
> +} catch (Exception exception) {
> +
>private final ScriptEngine scriptEngine;
>
>/**
> * Constructor
> * @param engine
> */
> - public ScriptEngineShellTable(String engine) {
> -this.engine = engine;
> -this.scriptEngine = scriptEngineFactory.getEngineByName(engine);
> + public
> @@ -0,0 +1,79 @@
> +/*
+1!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/79/files/0b99dad1aa7786eaf11028bf91fc1d272c189f0b#r78129943
> @@ -0,0 +1,79 @@
> +/*
> No changes to the notice file should be required as the copyright owner of
> the copied files is already the ASF.
Ah, good to know. Then I think we should be OK just to reformat and perhaps
tweak the style where applicable?
--
You are receiving this because you are
> @@ -51,9 +47,7 @@ public String evaluate(Object obj, String expression) {
> try {
>scriptEngine.put(getType(), obj);
>result = String.valueOf(scriptEngine.eval(expression));
> -} catch (Exception ex) {
> - //Ignore
> -}
> +} catch (Exception ignored) {}
>
> @@ -0,0 +1,79 @@
> +/*
Regarding licensing, according to
[this](http://www.apache.org/legal/src-headers.html#3party) and
[this](http://www.apache.org/legal/src-headers.html#notice), we should be good
to go. Although it is not the right header for software developed at Apache
(that is a
> I've just tried a rebased version of this branch on top of the shade one and
> everything works fine! :)
@nacx Nice! Agree with most of your comments - please feel free to add commits
to this branch with any changes you think would make sense.
Thanks for taking a look, and for testing!
--
> @@ -51,9 +47,7 @@ public String evaluate(Object obj, String expression) {
> try {
>scriptEngine.put(getType(), obj);
>result = String.valueOf(scriptEngine.eval(expression));
> -} catch (Exception ex) {
> - //Ignore
> -}
> +} catch (Exception ignored) {}
>
>private final ScriptEngine scriptEngine;
>
>/**
> * Constructor
> * @param engine
> */
> - public ScriptEngineShellTable(String engine) {
> -this.engine = engine;
> -this.scriptEngine = scriptEngineFactory.getEngineByName(engine);
> + public
> @@ -0,0 +1,79 @@
> +/*
> Do you plan to re-style the classes to align them to our coding style and
> change them to cleanup the code?
I think we should certainly align the code style; not sure about more
comprehensive re-writes, though. Agree that we should change the functionality
(esp.
>Could you give (a CLI built from) this a shot, on top of your "shade" changes
>to get the agentproxy to work again? It looks like it should be working
I've just tried a rebased version of this branch on top of the `shade` one and
everything works fine! :)
--
You are receiving this because
> @@ -0,0 +1,79 @@
> +/*
Do you plan to re-style the classes to align them to our coding style and
change them to cleanup the code? Or do you think it makes sense to leave them
unchanged in their own package?
--
You are receiving this because you are subscribed to this thread.
Reply to this
> @@ -51,9 +47,7 @@ public String evaluate(Object obj, String expression) {
> try {
>scriptEngine.put(getType(), obj);
>result = String.valueOf(scriptEngine.eval(expression));
> -} catch (Exception ex) {
> - //Ignore
> -}
> +} catch (Exception ignored) {}
>
>private final ScriptEngine scriptEngine;
>
>/**
> * Constructor
> * @param engine
> */
> - public ScriptEngineShellTable(String engine) {
> -this.engine = engine;
> -this.scriptEngine = scriptEngineFactory.getEngineByName(engine);
> + public
@nacx Could you give (a CLI built from) this a shot? It looks like it should be
working
```
jclouds> jclouds:compute-service-create --provider aws-ec2 --identity key
--credential secret --name test
Waiting for compute service with name: test.
jclouds> jclouds:location-list --provider aws-ec2
> + return null;
> +}
> + }
> +
> + /**
> + * Iterates through all bundles to get the available @link
> ScriptEngineFactory classes
> + * @return the names of the available ScriptEngineFactory classes
> + * @throws IOException
> + */
> + private List
> @@ -0,0 +1,79 @@
> +/*
Lots of cleanup and review still to go with these three classes
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Alternative to #78, but addressing the same issue.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-karaf/pull/79
-- Commit Summary --
* Try to use an OSGi-compliant way of loading JSR 223 script engines
-- File Changes --
M
25 matches
Mail list logo
