The suggestion sound very reasonable and actionable, so: +1
I suggest a JIRA issue is opened for this that references this thread, accompanied by the necessary sub-tasks (if any, and to be able to keep the patch files(s) as concise as possible per component [1]) regarding depending aspects (e.g. documentation). That way we can tie this in to future releases and not forget the other elements of the bigger picture. [1] large patch files are very hard to evaluate, and might lead to stale code improvements when not addressed in an appropriate time span. Best regards, Pierre Smits ORRTIZ.COM <http://www.orrtiz.com> OFBiz based solutions & services OFBiz Extensions Marketplace http://oem.ofbizci.net/oci-2/ On Wed, Jun 29, 2016 at 1:32 PM, Gavin Mabie <kwikst...@gmail.com> wrote: > +1 > > On Wed, Jun 29, 2016 at 1:10 PM, Amardeep Singh Jhajj < > amardeep.jh...@hotwaxsystems.com> wrote: > > > Thanks all for approval. I will start work on it. > > > > Regards, > > -- > > Amardeep Singh Jhajj > > www.hotwaxsystems.com > > > > On Tue, Jun 28, 2016 at 11:51 AM, Jacques Le Roux < > > jacques.le.r...@les7arts.com> wrote: > > > > > Fair enough, +1 > > > > > > Jacques > > > > > > > > > > > > Le 28/06/2016 à 07:59, Taher Alkhateeb a écrit : > > > > > >> +1 on approach > > >> On Jun 28, 2016 8:53 AM, "Pranay Pandey" < > > pranay.pan...@hotwaxsystems.com > > >> > > > >> wrote: > > >> > > >> +1 Amardeep, mentioned approach is looking reasonable to me. > > >>> > > >>> Best regards, > > >>> > > >>> Pranay Pandey > > >>> HotWax Systems > > >>> http://www.hotwaxsystems.com/ > > >>> > > >>> On Tue, Jun 28, 2016 at 11:21 AM, Amardeep Singh Jhajj < > > >>> amardeep.jh...@hotwaxsystems.com> wrote: > > >>> > > >>> Hello Jacques, > > >>>> > > >>>> Making branch is a good idea to handle regressions but I would > prefer > > to > > >>>> work in trunk itself. Here are the things we should consider before > > >>>> creating new branch: > > >>>> > > >>>> 1. Branch can be abandoned, I would prefer to have code changes in > > trunk > > >>>> > > >>> in > > >>> > > >>>> proper steps instead of merging the complete branch work later in > > trunk. > > >>>> 2. Currently, lot of changes is coming in trunk, so different branch > > >>>> need > > >>>> to updated regularly. Its also an additional work. > > >>>> > > >>>> To avoid regressions, we can do our work in granular level tasks for > > >>>> various functionality (wherever needed) and proper testing steps > will > > be > > >>>> there. The code changes will only be committed after thorough > testing. > > >>>> > > >>>> First, I would like to start with cleanup of inline javascripts so > > that > > >>>> > > >>> all > > >>> > > >>>> javascript code will only in js files which helps us to follow best > > >>>> practices of javascript. > > >>>> Here is the steps can be taken to do it: > > >>>> > > >>>> 1. Start with one component. For example: Accounting. > > >>>> 2. Pick its various functionality one by one where javascript is > added > > >>>> inline. > > >>>> 3. Do cleanup in it. > > >>>> 4. Do thorough testing for it with testing steps over ticket. > > >>>> > > >>>> Once the all cleanup (inline js cleanup) is done, we would have > > >>>> > > >>> javascript > > >>> > > >>>> files for further work. We can further break the js work by > > >>>> functionality > > >>>> (if needed) for improvements. > > >>>> > > >>>> The above process will assure us that nothing will break. I will not > > >>>> work > > >>>> on this effort alone, the team of developers having two or more year > > >>>> experience in javascript will work with me. > > >>>> > > >>>> I will create the parent ticket for selenium tests as well and we > will > > >>>> > > >>> see > > >>> > > >>>> how we can work on it. > > >>>> > > >>>> Thanks & Regards > > >>>> -- > > >>>> Amardeep Singh Jhajj > > >>>> www.hotwaxsystems.com > > >>>> > > >>>> On Sat, Jun 25, 2016 at 1:06 PM, Jacques Le Roux < > > >>>> jacques.le.r...@les7arts.com> wrote: > > >>>> > > >>>> Hi Amardeep, > > >>>>> > > >>>>> What I mean is we should be cautious. Hence the suggestion to > create > > a > > >>>>> branch for the refactoring. And to benefit from this opportunity to > > put > > >>>>> some Selenium tests related with js (eg: calendar - could also use > > >>>>> > > >>>> HTML5 > > >>> > > >>>> now -, autocompletion, lookups, dependent dropdowns). That would be > a > > >>>>> > > >>>> start > > >>>> > > >>>>> for Selenium, no needs to have tons of it. A Jira subtask would be > > >>>>> > > >>>> perfect. > > >>>> > > >>>>> Hope I'm clearer > > >>>>> > > >>>>> Thanks > > >>>>> > > >>>>> Jacques > > >>>>> > > >>>>> > > >>>>> > > >>>>> Le 24/06/2016 à 15:53, Amardeep Singh Jhajj a écrit : > > >>>>> > > >>>>> Thanks all for approval. > > >>>>>> > > >>>>>> Thanks Taher and Deepak for valuable suggestions. I will think on > > >>>>>> > > >>>>> them. > > >>> > > >>>> Jacques, > > >>>>>> > > >>>>>> I am bit confused by your reply, could you please elaborate your > > >>>>>> > > >>>>> point. > > >>> > > >>>> Regards, > > >>>>>> -- > > >>>>>> Amardeep Singh Jhajj > > >>>>>> > > >>>>>> > > >>>>>> On Fri, Jun 24, 2016 at 4:17 PM, Jacques Le Roux < > > >>>>>> jacques.le.r...@les7arts.com> wrote: > > >>>>>> > > >>>>>> Amardeep, > > >>>>>> > > >>>>>>> I agree with your points, I'd just request that we do that in a > > >>>>>>> > > >>>>>> branch > > >>> > > >>>> with a load of UI tests (good occasion to use Selenium). > > >>>>>>> Our js code is complex and fragile, I think notably at the global > > >>>>>>> variables in fieldlookup.js and selectall.js which are maybe hard > > to > > >>>>>>> avoid. > > >>>>>>> IMO those files are the main pains. > > >>>>>>> > > >>>>>>> Thanks > > >>>>>>> > > >>>>>>> Jacques > > >>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> Le 24/06/2016 à 09:20, Amardeep Singh Jhajj a écrit : > > >>>>>>> > > >>>>>>> Hello everyone, > > >>>>>>> > > >>>>>>>> Currently, OFBiz javascript code (except third party libraries) > is > > >>>>>>>> > > >>>>>>> not > > >>> > > >>>> written with the best practices which can cause following problems - > > >>>>>>>> > > >>>>>>>> 1. Increases the code maintenance effort. > > >>>>>>>> 2. Impact page performance. > > >>>>>>>> 3. Present not good examples to new contributors which leads to > > C/P > > >>>>>>>> > > >>>>>>> to > > >>> > > >>>> various areas of js code. > > >>>>>>>> > > >>>>>>>> Here are things we should do for cleanup and improvements in js > > >>>>>>>> > > >>>>>>> code. > > >>> > > >>>> 1. Remove unused javascript code and files if any. > > >>>>>>>> 2. Use best practices for javascript coding to improve > performance > > >>>>>>>> > > >>>>>>> (I > > >>> > > >>>> have > > >>>>>>>> listed some of it below). > > >>>>>>>> 3. Move utility js functions to one js file. > > >>>>>>>> 4. Remove deprecated code and use latest. For ex: We are still > > using > > >>>>>>>> "language='javascript'" attribute at script tag which is > > deprecated > > >>>>>>>> > > >>>>>>> a > > >>> > > >>>> years > > >>>>>>>> ago. > > >>>>>>>> 5. js should be loaded at bottom of the page, currently its in > > >>>>>>>> > > >>>>>>> Header. > > >>> > > >>>> Its > > >>>>>>>> a tedious task now to move it into footer because we have lot of > > js > > >>>>>>>> > > >>>>>>> code > > >>>> > > >>>>> inline in ftls. > > >>>>>>>> 6. js should not be written inline, it should be enough generic > to > > >>>>>>>> > > >>>>>>> be > > >>> > > >>>> in > > >>>> > > >>>>> minimum number of files and have generic code for doing the common > > >>>>>>>> > > >>>>>>> set > > >>> > > >>>> of > > >>>>>>>> operations over DOM. > > >>>>>>>> 7. Currently our macros of rendering pages has inline scripts, > > they > > >>>>>>>> > > >>>>>>> can > > >>>> > > >>>>> be > > >>>>>>>> moved to one macrorenderer.js with generic code as we can use > > >>>>>>>> > > >>>>>>> classes, > > >>> > > >>>> ids > > >>>>>>>> and data-attributes for doing any operation over html DOM. > > >>>>>>>> 8. After all cleanup work, we can think of build tool (like > grunt) > > >>>>>>>> > > >>>>>>> for > > >>> > > >>>> various javascript build tasks (minification, concatenation of > > >>>>>>>> > > >>>>>>> files) > > >>> > > >>>> if > > >>>> > > >>>>> needed. Its just a thought. > > >>>>>>>> > > >>>>>>>> I know its a huge effort and need to be done carefully. So > before > > >>>>>>>> > > >>>>>>> doing > > >>>> > > >>>>> any > > >>>>>>>> major changes, I would like to start work with first 4 points. > > >>>>>>>> > > >>>>>>>> Here is the list of some best practices to start with: > > >>>>>>>> > > >>>>>>>> 1. Use [] Instead of New Array() > > >>>>>>>> 2. Long list of variables? Omit the "Var" keyword and use commas > > >>>>>>>> instead. > > >>>>>>>> 3. Reduce global variables > > >>>>>>>> 4. Use explicit blocks > > >>>>>>>> 5. Start blocks on the same line > > >>>>>>>> 6. Always, Always Use Semicolons - Having said that, this is a > > very > > >>>>>>>> > > >>>>>>> bad > > >>>> > > >>>>> practice that can potentially lead to much bigger, and harder to > > >>>>>>>> > > >>>>>>> find, > > >>> > > >>>> issues. > > >>>>>>>> 7. Optimize loops. Avoid calculating the length of array in for > > loop > > >>>>>>>> iteration. > > >>>>>>>> 8. Avoid multiple redundant jQuery DOM manupulation by saving > > >>>>>>>> > > >>>>>>> reference > > >>>> > > >>>>> to > > >>>>>>>> any object. > > >>>>>>>> 9. Try to use meaningful comments. > > >>>>>>>> 10. Many more. > > >>>>>>>> > > >>>>>>>> Here are some links of best practices information- > > >>>>>>>> > > >>>>>>>> https://www.w3.org/wiki/JavaScript_best_practices > > >>>>>>>> > > >>>>>>>> > > http://www.codeproject.com/Articles/580165/JavaScript-Best-Practices > > >>> > > >>>> https://developer.yahoo.com/performance/rules.html > > >>>>>>>> > > >>>>>>>> If everyone agrees, I would like to start on this work. > > >>>>>>>> > > >>>>>>>> Please let me know your thoughts on it. > > >>>>>>>> > > >>>>>>>> Thanks and Regards > > >>>>>>>> -- > > >>>>>>>> Amardeep Singh Jhajj > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > > > > >
