I'm happy to put this work on hold for now. As I mentioned there are other benefits beyond just getters and setters -- but I guess the risk with JDK10 (I was unaware of the problems between Lombok and JDK10) may be unknown enough for us to take pause. Thanks for all your input!
-Aditya On Fri, Nov 9, 2018 at 9:52 AM Aditya Anchuri <aanch...@pivotal.io> wrote: > I apologize, I was slightly wrong earlier with regards to the the IntelliJ > Lombok plugin -- people will not need the IntelliJ plugin for their code to > compile -- but they will need to enable compile-time annotation processing > as an option. The plugin is a nice-to-have. > > > > On Fri, Nov 9, 2018 at 9:40 AM Kirk Lund <kl...@apache.org> wrote: > >> -1 Personally, I prefer to being able to see and manipulate ALL code. I >> hate autogenerated code, and I don't mind boilerplate code. When I see a >> class that has too many getters, then I see that as a sign that we need to >> do some refactoring and correct the design of that class. Using Lombok >> would hide too much and make it less obvious that we have Encapsulation >> and >> God Class problems to fix and change. >> >> Generated code also makes stepping through code with a debugger and doing >> performance engineering a nightmare. >> >> On Fri, Nov 9, 2018 at 9:07 AM, Anthony Baker <aba...@pivotal.io> wrote: >> >> > I was talking with Dale and he pointed me to this discussion: >> > https://github.com/jhipster/generator-jhipster/issues/398 < >> > https://github.com/jhipster/generator-jhipster/issues/398> >> > >> > I think it probably warrants more investigation (e.g. do the issues >> > decried on the the internet still exist?) before we adopt this. >> > >> > Anthony >> > >> > >> > > On Nov 9, 2018, at 9:00 AM, Jinmei Liao <jil...@pivotal.io> wrote: >> > > >> > > So users who wish to download our code will need to read some >> > documentation >> > > on how to setup IDEA/Eclipse before they can compile. It's not a >> simple >> > > "import and work with default IDEA setup" now. >> > > >> > > +0 on this. Personally I would prefer to bear with the extra >> > getter/setters >> > > than giving new contributors more headache at startup. >> > > >> > > On Thu, Nov 8, 2018 at 5:10 PM Udo Kohlmeyer <u...@apache.org> wrote: >> > > >> > >> As an informative comparison on conciseness offering same >> functionality: >> > >> >> > >> Java Code: >> > >> >> > >> import java.io.Serializable; import lombok.Getter; import >> > >> org.apache.geode.cache.configuration.CacheConfig.GatewayReceiver; >> > import >> > >> org.apache.geode.cache.configuration.DeclarableType; /** * This class >> > >> stores the arguments provided in the create >> > >> gateway-receiver command. */ @Getter public class >> > >> GatewayReceiverFunctionArgsimplements Serializable { >> > >> private static final long serialVersionUID = >> -5158224572470173267L; >> > >> private final BooleanmanualStart; private final IntegerstartPort; >> > private >> > >> final IntegerendPort; private final StringbindAddress; private final >> > >> IntegersocketBufferSize; private final >> IntegermaximumTimeBetweenPings; >> > >> private final String[]gatewayTransportFilters; private final >> > >> StringhostnameForSenders; private final BooleanifNotExists; public >> > >> GatewayReceiverFunctionArgs(GatewayReceiver configuration, Boolean >> > >> ifNotExists) { >> > >> this.manualStart = configuration.isManualStart(); >> this.startPort = >> > >> configuration.getStartPort() !=null ? >> > >> Integer.valueOf(configuration.getStartPort()) :null; this.endPort = >> > >> configuration.getEndPort() !=null ? >> > >> Integer.valueOf(configuration.getEndPort()) :null; this.bindAddress = >> > >> configuration.getBindAddress(); this.socketBufferSize = >> > >> configuration.getSocketBufferSize() !=null ? >> > >> Integer.valueOf(configuration.getSocketBufferSize()) :null; >> > >> this.maximumTimeBetweenPings = configuration. >> > getMaximumTimeBetweenPings() >> > >> !=null ? Integer.valueOf(configuration.getMaximumTimeBetweenPings()) >> > :null; >> > >> this.gatewayTransportFilters = configuration. >> > getGatewayTransportFilter() >> > >> !=null ? >> > >> >> configuration.getGatewayTransportFilter().stream().map(DeclarableType:: >> > getClassName) >> > >> .toArray(String[]::new) >> > >> :null; this.hostnameForSenders = >> > >> configuration.getHostnameForSenders(); this.ifNotExists = >> ifNotExists; >> > } >> > >> } >> > >> >> > >> The equivalent Kotlin definition is: >> > >> >> > >> import org.apache.geode.cache.configuration.CacheConfig >> > >> import org.apache.geode.cache.configuration.ClassWithParametersType >> > >> import java.io.Serializable >> > >> >> > >> data class GatewayReceiverFunctionArgs >> > >> @JvmOverloads private constructor(val manualStart: Boolean, val >> > startPort: >> > >> Int?, val endPort: Int?, val bindAddress: String, val >> socketBufferSize: >> > >> Int?, val maximumTimeBetweenPings: Int?, val gatewayTransportFilters: >> > >> Array<String>, val hostNameForSender: String, val ifNotExists: >> Boolean) >> > : >> > >> Serializable{ >> > >> >> > >> constructor(configuration: CacheConfig.GatewayReceiver, >> ifNotExists: >> > >> Boolean) : >> > >> this(configuration.isManualStart, >> > >> configuration.startPort?.toInt(), configuration.endPort?.toInt(), >> > >> configuration.bindAddress, configuration.socketBufferSize?.toInt(), >> > >> configuration.maximumTimeBetweenPings?.toInt(), >> > >> configuration.gatewayTransportFilter ?.map { classWithParametersType: >> > >> ClassWithParametersType-> classWithParametersType.className } >> > >> ?.toTypedArray<String>() >> > >> ?:emptyArray(), configuration.hostnameForSenders, >> > >> ifNotExists) >> > >> >> > >> >> > >> companion object { >> > >> @JvmStatic val serialVersionUID = -5158224572470173267L } >> > >> } >> > >> >> > >> >> > >> On 11/8/18 12:02, Aditya Anchuri wrote: >> > >>> I've only touched a few classes in my PR, but I feel like there's a >> lot >> > >>> more boilerplate floating around that can be removed. Having said >> > that, I >> > >>> agree with your point regarding Kotlin, but for the Java code I >> would >> > >> find >> > >>> Lombok pretty useful. Have included a link to the PR: >> > >>> >> > >>> https://github.com/apache/geode/pull/2815 >> > >>> >> > >>> -Aditya >> > >>> >> > >>> >> > >>> >> > >>> On Thu, Nov 8, 2018 at 11:24 AM Udo Kohlmeyer <u...@apache.org> >> wrote: >> > >>> >> > >>>> The Spring world/community are heavy users of Lombok. >> > >>>> >> > >>>> In essence it is "nice", BUT it does now add a new dependency on a >> > >>>> library that is to provide functionality that developers should >> > provide. >> > >>>> IJ Idea does provide support for Lombok. >> > >>>> >> > >>>> I have not yet seen any code bloat that Lombok could reduce for us. >> > >>>> Also, the reduction is only in terms of "visible", the compiled >> class >> > >>>> might be more verbose. >> > >>>> >> > >>>> Kotlin on the other hand, as some of the boilerplate code built in >> as >> > a >> > >>>> language feature. I prefer that over choosing a library, that might >> > have >> > >>>> compatibility issues in the future. >> > >>>> >> > >>>> Also, Kotlin's conciseness is also a language feature rather than >> > >>>> library plugin. I've also seen cases where compiled Java was larger >> > than >> > >>>> the equivalent compiled Kotlin code. >> > >>>> >> > >>>> --Udo >> > >>>> >> > >>>> >> > >>>> On 11/8/18 10:31, Aditya Anchuri wrote: >> > >>>>> Hi everyone, >> > >>>>> >> > >>>>> I am considering adding Lombok as a compile-time dependency ( >> > >>>>> https://projectlombok.org/) so we can reduce the amount of >> > boilerplate >> > >>>> code >> > >>>>> and reduce the size of some of our classes. I have a small proof >> of >> > >>>> concept >> > >>>>> PR ready to go. Before I do so, I want to find out if people have >> > tried >> > >>>> it >> > >>>>> before and how they feel about it, especially when used with IDEs >> > like >> > >>>>> IntelliJ and/or Eclipse? >> > >>>>> >> > >>>>> Thanks, >> > >>>>> -Aditya >> > >>>>> >> > >>>> >> > >> >> > >> >> > > >> > > -- >> > > Cheers >> > > >> > > Jinmei >> > >> > >> >
