I agree with you Pierre. I am working on planning of this work and will break work logically for different components to keep it very simple. I will share ticket here.
Thanks & Regards, -- Amardeep Singh Jhajj www.hotwaxsystems.com On Wed, Jun 29, 2016 at 5:21 PM, Pierre Smits <pierre.sm...@gmail.com> wrote: > 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 > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > > > > > > > >
