brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aled Sage <aled.s...@gmail.com>
Subject Re: [PROPOSAL] Remove groovy dependency/support
Date Fri, 24 Mar 2017 18:28:14 GMT
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 <martin.harris@cloudsoftcorp.com>
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 <geoff.macartney@cloudsoftcorp.com
>>> wrote:
>>> +1 certainly sounds like a good plan
>>>
>>> On Tue, 7 Mar 2017 at 01:17 Alex Heneveld <alex.heneveld@cloudsoftcorp.com
>>> 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


Mime
View raw message