Hi all,

I've deprecated the groovy'isms in our code [1], as proposed for 0.11.0.

Aled

[1] https://github.com/apache/brooklyn-server/pull/611



On 07/03/2017 10:52, Svetoslav Neykov wrote:
+1

All for it. It's just dead code at this point so highly likely to have bugs.
The wins won't be too visible though. It' just a few internal classes that know 
about Groovy. There's no dependency on the groovy compiler apart from a handful 
of tests in core. The IDEs are fine ignoring them even now.

* Reduce our binary distribution by over 7MB

Can't happen unless we drop the "console" in the REST API. We can still use 
Groovy there. Any scriptable JVM language would work but don't see a point in switching.

Svet.



On 7.03.2017 г., at 11:23, Martin Harris <[email protected]> 
wrote:

Would this include the removal of the Groovy console? I'd kinda miss that
:-(

Cheers

M

On 7 March 2017 at 09:10, Geoff Macartney <[email protected]
wrote:
+1 certainly sounds like a good plan

On Tue, 7 Mar 2017 at 01:17 Alex Heneveld <[email protected]
wrote:

+1

Minor comments:

2. For a "MethodEffector", the effector invocation goes through
  `GroovyJavaMethods.invokeMethodOnMetaClass`. This calls into groovy
  code to find the method that matches the given arguments.
Reflections.findMethodMaybe and invokeMethodWithArgs might be drop-in
replacements

one can often supply a Groovy closure instance (e.g. as a config key
value) and
have it automatically converted to Predicate/Function/etc
Java 8-isms should let us overhaul this.  As a first step in 0.11 for
good measure we should log.warn if a Closure is being supplied.

--A



On 06/03/2017 19:01, Aled Sage wrote:
Hi all,

I propose we deprecate and then remove the remaining groovy code from
Brooklyn: deprecate for 0.11.0, and delete a couple of releases later.

I think we should treat Groovy like any other JVM-based language that
users might want to use: it's the user's responsibility; we present a
pure Java API that those languages can call.

I'd estimate the first stage (deprecation in 0.11.0) as being just a
few hours work.

_*Advantages of Deleting this Code*_

* Simplify our code base
   (with all the usual advantages of understandability, simpler
   refactoring, etc).
* Make support easier
   (luckily no-one has asked any Groovy questions in years! Do we
   really want to fix any bugs that Groovy users hit?!)
* Make it easier for (Java) developers:
     o More understandable e.g. the black magic of how "MethodEffector"
       works.
     o Easier IDE setup (e.g. don't need to worry about .groovy files,
       or risk breaking them if you ignore them)
* Simplify our build process
* Reduce our binary distribution by over 7MB


_*Background*_
The most early versions of Brooklyn (before it joined Apache) were
written in Groovy. We grew to regret that technology choice, and
switched to pure Java instead. We also now strongly recommend users to
focus on YAML-based blueprints whereever possible, which makes things
like Groovy support even more redundant.


_*Current Usage*_
Groovy is (unfortunately) still used under-the-covers in a few places:

1. Groovy's `ObservableList` is used under-the-covers by
   `LocalEntityManager.entities`.
2. For a "MethodEffector", the effector invocation goes through
   `GroovyJavaMethods.invokeMethodOnMetaClass`. This calls into groovy
   code to find the method that matches the given arguments.
3. `GroovyJavaMethods` is used for some groovy'isms (Groovy Truth
   primarily).

Groovy's closures have special support in various places - e.g. some
methods are overloaded to accept a Closure instead of a
Runnable/Callable/Function/Predicate. Also, the `TypeCoercions` means
that one can often supply a Groovy closure instance (e.g. as a config
key value) and have it automatically converted to
Predicate/Function/etc.
I think our build does special groovy stuff (e.g. there are groovy
test classes, which explicitly test the Closure support).


_*Next Steps*_
Assuming we agree...

For 0.11.0:

* Deprecate all methods that take a Groovy parameter type (e.g.
   Closure, groovy.time.TimeDuration, etc).
* Deprecate `GroovyJavaMethods` and the other classes in the
   `brooklyn-utils-groovy` module.
* Include in the release notes that this is deprecated, and Groovy
   will be removed from Brooklyn in a future release.

Subsequently:

* Remove internal uses of Groovy, at our leisure.

In some future release:

* Delete all the deprecated methods and utilities, the groovy
   dependency, the .groovy test classes, and any other mentions of it
   from our poms/build.

I'd estimate the first stage (for 0.11.0) as being just a few hours
work.
Aled





--
Martin Harris
Lead Software Engineer
Cloudsoft Corporation Ltd
www.cloudsoftcorp.com
Mobile: +44 (0)7989 047-855

Reply via email to