Hello Everyone,
So while trying to refactor Start.java, I see uglier code as I dig deeper. One of the annoying things I've seen so far is the introduction of jsvc through the class org.ofbiz.base.start.CommonsDaemonStart. The dependency from CommonsDaemonStart.java to Start.java is making the code ugly and exposed not to mention some other technical problems as discussed in https://issues.apache.org/jira/browse/OFBIZ-5710 I want to know whether the community is using this class and there is valuable use for it, or whether we can just delete it. I believe we can reintroduce jsvc in a much cleaner way in the future. Thank you in advance for your feedback. Taher Alkhateeb |
Administrator
|
Hi Taher,
Jsvc sounds like a good thing to have. So if you envision a better to way to have it in OFBiz, please share in a new Jira with a ref to 5710 Thanks Jacques Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : > Hello Everyone, > > So while trying to refactor Start.java, I see uglier code as I dig deeper. > One of the annoying things I've seen so far is the introduction of jsvc > through the class org.ofbiz.base.start.CommonsDaemonStart. > > The dependency from CommonsDaemonStart.java to Start.java is making the > code ugly and exposed not to mention some other technical problems as > discussed in https://issues.apache.org/jira/browse/OFBIZ-5710 > > I want to know whether the community is using this class and there is > valuable use for it, or whether we can just delete it. I believe we can > reintroduce jsvc in a much cleaner way in the future. > > Thank you in advance for your feedback. > > Taher Alkhateeb > |
Hi Jacque,
I suspect that jsvc is half-cooked and is not really implemented. You can't find commons-daemon-*.jar anywhere in the code base. The only thing that exists is CommonsDaemonStart.java which is exposing Start.start(), Start.shutdownServer() and Start.init() and is not fully implemented (destroy() does nothing). My objective right now is not to implement jsvc (although a good idea) but to clean up the code. It is really nasty to expose Start.java by calling Start.getInstance().init(args) for example. So my suggestion is, if no one is actually using or depending on CommonsDaemonStart.java (I suspect no one is using it) then we are better off deleting it. This makes the refactoring of Start.java much better and actually allows for better implementation of jsvc in the future if needed. So my recommendation for better cleaner code is to simply delete CommonsDaemonStart.java due to poor code and my suspicion that it is not used. Otherwise, I'll have to try and refactor _around_ it. Not sure if we should vote on this, or wait for more feedback, or just drop it? Appreciate any feedback. Taher Alkhateeb On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < [hidden email]> wrote: > Hi Taher, > > Jsvc sounds like a good thing to have. So if you envision a better to way > to have it in OFBiz, please share in a new Jira with a ref to 5710 > > Thanks > > Jacques > > > Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : > >> Hello Everyone, >> >> So while trying to refactor Start.java, I see uglier code as I dig deeper. >> One of the annoying things I've seen so far is the introduction of jsvc >> through the class org.ofbiz.base.start.CommonsDaemonStart. >> >> The dependency from CommonsDaemonStart.java to Start.java is making the >> code ugly and exposed not to mention some other technical problems as >> discussed in https://issues.apache.org/jira/browse/OFBIZ-5710 >> >> I want to know whether the community is using this class and there is >> valuable use for it, or whether we can just delete it. I believe we can >> reintroduce jsvc in a much cleaner way in the future. >> >> Thank you in advance for your feedback. >> >> Taher Alkhateeb >> >> > |
Administrator
|
Actually this is what I think too. Adam never completed the work and it's useless as is. Maybe he has his own usage of it, but for a reason did not
contribute it. Anyway, I see no problem removing it, it seems Adam and others are also not concerned. Jacques Le 06/05/2016 à 12:11, Taher Alkhateeb a écrit : > Hi Jacque, > > I suspect that jsvc is half-cooked and is not really implemented. You can't > find commons-daemon-*.jar anywhere in the code base. The only thing that > exists is CommonsDaemonStart.java which is exposing Start.start(), > Start.shutdownServer() and Start.init() and is not fully implemented > (destroy() does nothing). > > My objective right now is not to implement jsvc (although a good idea) but > to clean up the code. It is really nasty to expose Start.java by calling > Start.getInstance().init(args) for example. So my suggestion is, if no one > is actually using or depending on CommonsDaemonStart.java (I suspect no one > is using it) then we are better off deleting it. This makes the refactoring > of Start.java much better and actually allows for better implementation of > jsvc in the future if needed. > > So my recommendation for better cleaner code is to simply delete > CommonsDaemonStart.java due to poor code and my suspicion that it is not > used. Otherwise, I'll have to try and refactor _around_ it. Not sure if we > should vote on this, or wait for more feedback, or just drop it? Appreciate > any feedback. > > Taher Alkhateeb > > > > On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < > [hidden email]> wrote: > >> Hi Taher, >> >> Jsvc sounds like a good thing to have. So if you envision a better to way >> to have it in OFBiz, please share in a new Jira with a ref to 5710 >> >> Thanks >> >> Jacques >> >> >> Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : >> >>> Hello Everyone, >>> >>> So while trying to refactor Start.java, I see uglier code as I dig deeper. >>> One of the annoying things I've seen so far is the introduction of jsvc >>> through the class org.ofbiz.base.start.CommonsDaemonStart. >>> >>> The dependency from CommonsDaemonStart.java to Start.java is making the >>> code ugly and exposed not to mention some other technical problems as >>> discussed in https://issues.apache.org/jira/browse/OFBIZ-5710 >>> >>> I want to know whether the community is using this class and there is >>> valuable use for it, or whether we can just delete it. I believe we can >>> reintroduce jsvc in a much cleaner way in the future. >>> >>> Thank you in advance for your feedback. >>> >>> Taher Alkhateeb >>> >>> |
I second this approach as a general rule, if applied carefully and wisely
(as it is in this specific case). By removing incomplete code, the person working on the refactoring will have a simplified system to work with in the refactoring; the removed code will always be available in the history of the revision control system and the persons interested in completing the support of the removed feature will have a chance to check the code out and refactor/complete it. Jacopo On Tue, May 10, 2016 at 7:03 PM, Jacques Le Roux < [hidden email]> wrote: > Actually this is what I think too. Adam never completed the work and it's > useless as is. Maybe he has his own usage of it, but for a reason did not > contribute it. > > Anyway, I see no problem removing it, it seems Adam and others are also > not concerned. > > Jacques > > > Le 06/05/2016 à 12:11, Taher Alkhateeb a écrit : > >> Hi Jacque, >> >> I suspect that jsvc is half-cooked and is not really implemented. You >> can't >> find commons-daemon-*.jar anywhere in the code base. The only thing that >> exists is CommonsDaemonStart.java which is exposing Start.start(), >> Start.shutdownServer() and Start.init() and is not fully implemented >> (destroy() does nothing). >> >> My objective right now is not to implement jsvc (although a good idea) but >> to clean up the code. It is really nasty to expose Start.java by calling >> Start.getInstance().init(args) for example. So my suggestion is, if no one >> is actually using or depending on CommonsDaemonStart.java (I suspect no >> one >> is using it) then we are better off deleting it. This makes the >> refactoring >> of Start.java much better and actually allows for better implementation of >> jsvc in the future if needed. >> >> So my recommendation for better cleaner code is to simply delete >> CommonsDaemonStart.java due to poor code and my suspicion that it is not >> used. Otherwise, I'll have to try and refactor _around_ it. Not sure if we >> should vote on this, or wait for more feedback, or just drop it? >> Appreciate >> any feedback. >> >> Taher Alkhateeb >> >> >> >> On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < >> [hidden email]> wrote: >> >> Hi Taher, >>> >>> Jsvc sounds like a good thing to have. So if you envision a better to way >>> to have it in OFBiz, please share in a new Jira with a ref to 5710 >>> >>> Thanks >>> >>> Jacques >>> >>> >>> Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : >>> >>> Hello Everyone, >>>> >>>> So while trying to refactor Start.java, I see uglier code as I dig >>>> deeper. >>>> One of the annoying things I've seen so far is the introduction of jsvc >>>> through the class org.ofbiz.base.start.CommonsDaemonStart. >>>> >>>> The dependency from CommonsDaemonStart.java to Start.java is making the >>>> code ugly and exposed not to mention some other technical problems as >>>> discussed in https://issues.apache.org/jira/browse/OFBIZ-5710 >>>> >>>> I want to know whether the community is using this class and there is >>>> valuable use for it, or whether we can just delete it. I believe we can >>>> reintroduce jsvc in a much cleaner way in the future. >>>> >>>> Thank you in advance for your feedback. >>>> >>>> Taher Alkhateeb >>>> >>>> >>>> > |
Hi Jacques, Jacopo and everyone,
I have a big patch (about 1,500 lines) that among other things deletes CommonsDaemonStart and fully refactors the Start component. I need help in checking this patch. The system seems to be running smoothly and all tests pass on my computer. A few more eyeballs on this would help though. Thank you for your help! Taher Alkhateeb -----Original Message----- From: Jacopo Cappellato [mailto:[hidden email]] Sent: 17 May 2016 10:54 To: [hidden email] Subject: Re: Deleting CommonsDaemonStart.java I second this approach as a general rule, if applied carefully and wisely (as it is in this specific case). By removing incomplete code, the person working on the refactoring will have a simplified system to work with in the refactoring; the removed code will always be available in the history of the revision control system and the persons interested in completing the support of the removed feature will have a chance to check the code out and refactor/complete it. Jacopo On Tue, May 10, 2016 at 7:03 PM, Jacques Le Roux < [hidden email]> wrote: > Actually this is what I think too. Adam never completed the work and > it's useless as is. Maybe he has his own usage of it, but for a reason > did not contribute it. > > Anyway, I see no problem removing it, it seems Adam and others are > also not concerned. > > Jacques > > > Le 06/05/2016 à 12:11, Taher Alkhateeb a écrit : > >> Hi Jacque, >> >> I suspect that jsvc is half-cooked and is not really implemented. You >> can't find commons-daemon-*.jar anywhere in the code base. The only >> thing that exists is CommonsDaemonStart.java which is exposing >> Start.start(), >> Start.shutdownServer() and Start.init() and is not fully implemented >> (destroy() does nothing). >> >> My objective right now is not to implement jsvc (although a good >> idea) but to clean up the code. It is really nasty to expose >> Start.java by calling >> Start.getInstance().init(args) for example. So my suggestion is, if >> no one is actually using or depending on CommonsDaemonStart.java (I >> suspect no one is using it) then we are better off deleting it. This >> makes the refactoring of Start.java much better and actually allows >> for better implementation of jsvc in the future if needed. >> >> So my recommendation for better cleaner code is to simply delete >> CommonsDaemonStart.java due to poor code and my suspicion that it is >> not used. Otherwise, I'll have to try and refactor _around_ it. Not >> sure if we should vote on this, or wait for more feedback, or just drop it? >> Appreciate >> any feedback. >> >> Taher Alkhateeb >> >> >> >> On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < >> [hidden email]> wrote: >> >> Hi Taher, >>> >>> Jsvc sounds like a good thing to have. So if you envision a better >>> to way to have it in OFBiz, please share in a new Jira with a ref to >>> 5710 >>> >>> Thanks >>> >>> Jacques >>> >>> >>> Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : >>> >>> Hello Everyone, >>>> >>>> So while trying to refactor Start.java, I see uglier code as I dig >>>> deeper. >>>> One of the annoying things I've seen so far is the introduction of >>>> jsvc through the class org.ofbiz.base.start.CommonsDaemonStart. >>>> >>>> The dependency from CommonsDaemonStart.java to Start.java is making >>>> the code ugly and exposed not to mention some other technical >>>> problems as discussed in >>>> https://issues.apache.org/jira/browse/OFBIZ-5710 >>>> >>>> I want to know whether the community is using this class and there >>>> is valuable use for it, or whether we can just delete it. I believe >>>> we can reintroduce jsvc in a much cleaner way in the future. >>>> >>>> Thank you in advance for your feedback. >>>> >>>> Taher Alkhateeb >>>> >>>> >>>> > |
Hi Taher,
I tried to understand and comment your patch, but my apologies it's too technical for me ! I'm sure on one point, if some other technical guys don't raise any alert, it's that you proposition goes on the good way ;) If I can, I will try to test some configuration case. Nicolas Le 22/05/2016 09:59, Taher Alkhateeb a écrit : > Hi Jacques, Jacopo and everyone, > > I have a big patch (about 1,500 lines) that among other things deletes CommonsDaemonStart and fully refactors the Start component. > > I need help in checking this patch. The system seems to be running smoothly and all tests pass on my computer. A few more eyeballs on this would help though. > > Thank you for your help! > > Taher Alkhateeb > > -----Original Message----- > From: Jacopo Cappellato [mailto:[hidden email]] > Sent: 17 May 2016 10:54 > To: [hidden email] > Subject: Re: Deleting CommonsDaemonStart.java > > I second this approach as a general rule, if applied carefully and wisely (as it is in this specific case). > By removing incomplete code, the person working on the refactoring will have a simplified system to work with in the refactoring; the removed code will always be available in the history of the revision control system and the persons interested in completing the support of the removed feature will have a chance to check the code out and refactor/complete it. > > Jacopo > > On Tue, May 10, 2016 at 7:03 PM, Jacques Le Roux < [hidden email]> wrote: > >> Actually this is what I think too. Adam never completed the work and >> it's useless as is. Maybe he has his own usage of it, but for a reason >> did not contribute it. >> >> Anyway, I see no problem removing it, it seems Adam and others are >> also not concerned. >> >> Jacques >> >> >> Le 06/05/2016 à 12:11, Taher Alkhateeb a écrit : >> >>> Hi Jacque, >>> >>> I suspect that jsvc is half-cooked and is not really implemented. You >>> can't find commons-daemon-*.jar anywhere in the code base. The only >>> thing that exists is CommonsDaemonStart.java which is exposing >>> Start.start(), >>> Start.shutdownServer() and Start.init() and is not fully implemented >>> (destroy() does nothing). >>> >>> My objective right now is not to implement jsvc (although a good >>> idea) but to clean up the code. It is really nasty to expose >>> Start.java by calling >>> Start.getInstance().init(args) for example. So my suggestion is, if >>> no one is actually using or depending on CommonsDaemonStart.java (I >>> suspect no one is using it) then we are better off deleting it. This >>> makes the refactoring of Start.java much better and actually allows >>> for better implementation of jsvc in the future if needed. >>> >>> So my recommendation for better cleaner code is to simply delete >>> CommonsDaemonStart.java due to poor code and my suspicion that it is >>> not used. Otherwise, I'll have to try and refactor _around_ it. Not >>> sure if we should vote on this, or wait for more feedback, or just drop it? >>> Appreciate >>> any feedback. >>> >>> Taher Alkhateeb >>> >>> >>> >>> On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < >>> [hidden email]> wrote: >>> >>> Hi Taher, >>>> Jsvc sounds like a good thing to have. So if you envision a better >>>> to way to have it in OFBiz, please share in a new Jira with a ref to >>>> 5710 >>>> >>>> Thanks >>>> >>>> Jacques >>>> >>>> >>>> Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : >>>> >>>> Hello Everyone, >>>>> So while trying to refactor Start.java, I see uglier code as I dig >>>>> deeper. >>>>> One of the annoying things I've seen so far is the introduction of >>>>> jsvc through the class org.ofbiz.base.start.CommonsDaemonStart. >>>>> >>>>> The dependency from CommonsDaemonStart.java to Start.java is making >>>>> the code ugly and exposed not to mention some other technical >>>>> problems as discussed in >>>>> https://issues.apache.org/jira/browse/OFBIZ-5710 >>>>> >>>>> I want to know whether the community is using this class and there >>>>> is valuable use for it, or whether we can just delete it. I believe >>>>> we can reintroduce jsvc in a much cleaner way in the future. >>>>> >>>>> Thank you in advance for your feedback. >>>>> >>>>> Taher Alkhateeb >>>>> >>>>> >>>>> |
Hi Nicolas,
Thank you for helping out, I got support from Jacopo and Gil which made me comfortable enough in committing. Essentially, although this is a large commit, there is zero change in ofbiz behavior, I'm just restructuring the code to make more readable, flexible and less brittle. If you want to really help out forget the code, just make all kinds of test scenarios by running java -jar ofbiz.jar with multiple commands and variations and try to break it. Compare that with pre-commit and see if anything is off. My testing, as well as Gil and Jacopo seems to be okay and I'll keep a close watch for a while. FYI on the technical side one of the problems I discovered is that the String[] args that are passed to the main method go everywhere (and I really mean everywhere!!) which is a terrible, terrible design. So I isolated them from the rest of the framework by creating an adapter interface. This is the main reason why I am now able to refactor so much after I eliminated those damn args that were jumping in my face everywhere I look. Cheers Taher Alkhateeb On Tue, May 24, 2016 at 10:34 PM, Nicolas Malin <[hidden email]> wrote: > Hi Taher, > > I tried to understand and comment your patch, but my apologies it's too > technical for me ! I'm sure on one point, if some other technical guys > don't raise any alert, it's that you proposition goes on the good way ;) > > If I can, I will try to test some configuration case. > > Nicolas > > Le 22/05/2016 09:59, Taher Alkhateeb a écrit : > >> Hi Jacques, Jacopo and everyone, >> >> I have a big patch (about 1,500 lines) that among other things deletes >> CommonsDaemonStart and fully refactors the Start component. >> >> I need help in checking this patch. The system seems to be running >> smoothly and all tests pass on my computer. A few more eyeballs on this >> would help though. >> >> Thank you for your help! >> >> Taher Alkhateeb >> >> -----Original Message----- >> From: Jacopo Cappellato [mailto:[hidden email]] >> Sent: 17 May 2016 10:54 >> To: [hidden email] >> Subject: Re: Deleting CommonsDaemonStart.java >> >> I second this approach as a general rule, if applied carefully and wisely >> (as it is in this specific case). >> By removing incomplete code, the person working on the refactoring will >> have a simplified system to work with in the refactoring; the removed code >> will always be available in the history of the revision control system and >> the persons interested in completing the support of the removed feature >> will have a chance to check the code out and refactor/complete it. >> >> Jacopo >> >> On Tue, May 10, 2016 at 7:03 PM, Jacques Le Roux < >> [hidden email]> wrote: >> >> Actually this is what I think too. Adam never completed the work and >>> it's useless as is. Maybe he has his own usage of it, but for a reason >>> did not contribute it. >>> >>> Anyway, I see no problem removing it, it seems Adam and others are >>> also not concerned. >>> >>> Jacques >>> >>> >>> Le 06/05/2016 à 12:11, Taher Alkhateeb a écrit : >>> >>> Hi Jacque, >>>> >>>> I suspect that jsvc is half-cooked and is not really implemented. You >>>> can't find commons-daemon-*.jar anywhere in the code base. The only >>>> thing that exists is CommonsDaemonStart.java which is exposing >>>> Start.start(), >>>> Start.shutdownServer() and Start.init() and is not fully implemented >>>> (destroy() does nothing). >>>> >>>> My objective right now is not to implement jsvc (although a good >>>> idea) but to clean up the code. It is really nasty to expose >>>> Start.java by calling >>>> Start.getInstance().init(args) for example. So my suggestion is, if >>>> no one is actually using or depending on CommonsDaemonStart.java (I >>>> suspect no one is using it) then we are better off deleting it. This >>>> makes the refactoring of Start.java much better and actually allows >>>> for better implementation of jsvc in the future if needed. >>>> >>>> So my recommendation for better cleaner code is to simply delete >>>> CommonsDaemonStart.java due to poor code and my suspicion that it is >>>> not used. Otherwise, I'll have to try and refactor _around_ it. Not >>>> sure if we should vote on this, or wait for more feedback, or just drop >>>> it? >>>> Appreciate >>>> any feedback. >>>> >>>> Taher Alkhateeb >>>> >>>> >>>> >>>> On Fri, May 6, 2016 at 11:45 AM, Jacques Le Roux < >>>> [hidden email]> wrote: >>>> >>>> Hi Taher, >>>> >>>>> Jsvc sounds like a good thing to have. So if you envision a better >>>>> to way to have it in OFBiz, please share in a new Jira with a ref to >>>>> 5710 >>>>> >>>>> Thanks >>>>> >>>>> Jacques >>>>> >>>>> >>>>> Le 05/05/2016 à 17:59, Taher Alkhateeb a écrit : >>>>> >>>>> Hello Everyone, >>>>> >>>>>> So while trying to refactor Start.java, I see uglier code as I dig >>>>>> deeper. >>>>>> One of the annoying things I've seen so far is the introduction of >>>>>> jsvc through the class org.ofbiz.base.start.CommonsDaemonStart. >>>>>> >>>>>> The dependency from CommonsDaemonStart.java to Start.java is making >>>>>> the code ugly and exposed not to mention some other technical >>>>>> problems as discussed in >>>>>> https://issues.apache.org/jira/browse/OFBIZ-5710 >>>>>> >>>>>> I want to know whether the community is using this class and there >>>>>> is valuable use for it, or whether we can just delete it. I believe >>>>>> we can reintroduce jsvc in a much cleaner way in the future. >>>>>> >>>>>> Thank you in advance for your feedback. >>>>>> >>>>>> Taher Alkhateeb >>>>>> >>>>>> >>>>>> >>>>>> > |
Free forum by Nabble | Edit this page |