Dear all, Here are two updates about lgtm:
1. Just few days after our discussion, GitHub announced that they acquired Semmle (the company that made lgtm). So lgtm is now part of GitHub. (See https://github.blog/2019-09-18-github-welcomes-semmle/) 2. Apache INFRA now has lgtm installed in the Apache GitHub organization account. We can enable the lgtm pull request integration on SINGA repo if everyone agrees. Thanks Moaz On 2019/09/16 04:20:43, Moaz Reyad <[email protected]> wrote: > Thank you all for participating in the discussion. It is nice that everyone > agrees that code quality is important. > > Here are some comments: > > 1. It seems that everyone agrees on cpplint and pylint, but no one > discussed lgtm. Do you agree on using also the lgtm tool which is proposed > in [1] ? or do you think it is not needed and the cpplint and pylint is > enough? Note that there are lgtm badges that can be added to SINGA readme > on Github and acts as a certificate of the code quality that is visible to > everyone. While cpplint and pylint are not as visible as lgtm. Also lgtm > seems to report other problems that cpplint and pylint do not recognize. > Please share your ideas about the lgtm tool and SINGA-484 [1] issue. > > 2. We need to separate the discussion of the problem of future code quality > and the problem of current code quality. Enforcing code quality in future > is great, but in my opinion the team must take some actions also for the > current code. There are hundreds of issues reported by the three tools as > mentioned in my previous email. Not all of them are important of course, > but some percentage of them need to be fixed. > > 3. For future code quality, it is a good idea to use continuous integration > to run static code checking (cpplint, pylint and also RAT). I can open a > ticket in jira for this and anyone is welcome to implement it (after > finishing the more urgent issues with Travis CI). But cpplint and pylint > test will show too many errors already on the current code in the master > branch. This is why I think we need to solve the problem of current code > quality before enforcing future code quality. > > 4. The official site of SINGA is [2]. When the user navigates to "How to > Contribute Code" link, he will go to [3]. The link that was mention in [4] > is not accessible from [2]. The English site was moved to the root as the > default language [5], so no need to use "/en" folder in the path. If you > use the latest site build script [6], it will not create "/en" folder. This > means the link [4] was not generated by the latest site build script [6]. > > 5. The code style instructions in [4] can be useful only for code style > issues, but all other code problems (such as those hundreds of issues > reported by the three tools) are not only about code style. Check the LGTM > alerts for SINGA [7] for example, they are not code style or readability > issues. (e.g unused variables, unreachable code, ..) > > best regards, > Moaz > > [1] https://issues.apache.org/jira/projects/SINGA/issues/SINGA-484 > [2] http://singa.apache.org > [3] http://singa.apache.org/develop/contribute-code.html > [4] http://singa.apache.org/en/develop/contribute-code.html > [5] https://issues.apache.org/jira/projects/SINGA/issues/SINGA-436 > [6] https://github.com/apache/incubator-singa/blob/master/doc/build.sh > [7] https://lgtm.com/projects/g/apache/incubator-singa/alerts/?mode=list > > On Sun, Sep 15, 2019 at 5:02 PM Yeung Sai Ho <[email protected]> wrote: > > > P.S. Therefore, tools like cpplint and pylint are useful and can be > > integrated into CI process that can check code automatically. > > > > 從我的 iPhone 傳送 > > > > Yeung Sai Ho <[email protected]<mailto:[email protected]>> 於 2019年9月15日 > > 下午10:18 寫道： > > > > > > Dear Prof. Moaz Reyad, > > > > > > > > Nice to discuss with you, I am currently with NUS as a research fellow, > > and working with my peer co-workers (e.g. shicong/dcslin, joddly, shicheng, > > wanqi, pinpom, etc.) on SINGA leaded by our boss. I am Chris Yeung and this > > is my first time to join the discussion here. > > > > > > > > I agree with you. As a researcher and developer, I totally feel the same > > since code quality is very important. The code analysis tool could save our > > time and improve coding quality. > > > > > > > > In this email, please let me share my own personal view, concerning some > > recent daily practice in our local team which is currently using to enhance > > code quality: > > > > > > > > We are currently using the tools like cpplint and pylint to check and > > reformat the code so that we make sure our code is in compliance with the > > google style. > > > > > > > > Concerning our SINGA website, the new developers are suggested to read the > > following SINGA doc web page before they join developing the SINGA, which > > gives a complete and useful guide for their development: > > > > https://singa.apache.org/en/develop/contribute-code.html > > > > > > > > The above website recommends a very simple approach to enforce the Google > > coding styles: We can use the VS Code editor ( > > https://code.visualstudio.com) and set the linting and formating tools. > > First, we need to install the C/C++ extension and python extension. Then, > > we can edit the seetings.json as: > > > > "editor.formatOnSave": true, > > > > "python.formatting.provider": "yapf", > > > > "python.formatting.yapfArgs": [ > > > > "--style", > > > > "{based_on_style: google}" > > > > ], > > > > "python.linting.enabled": true, > > > > "python.linting.lintOnSave": true, > > > > "C_Cpp.clang_format_style": "Google" > > > > > > > > A reference settings.json file can be found here: > > > > https://gist.github.com/nudles/3d23cfb6ffb30ca7636c45fe60278c55 > > > > > > > > The developers are suggested to fix the format errors before submitting > > the PRs (pull requests). > > > > > > > > In addition to the code quality improving tool, we also have the tools for > > adding documentations. The procedures to contribute for the documentation > > is in our SINGA doc web page: > > > > https://singa.apache.org/en/develop/contribute-docs.html > > > > > > > > In our recent development, we always perform peer discussion in the local > > team, formally meeting with our boss almost everyday, and seek advise from > > our boss whenever any problems arise. We learn from each other and improve > > ourselves gradually. > > > > > > > > The above is just to share my own personal view, concerning our recent > > experience that also concerns code quality. Thank you very much for > > listening! > > > > > > > > Best Regards, > > > > Research Fellow > > > > Chris YEUNG Sai Ho ( > > https://scholar.google.com.sg/citations?user=9XAJsd0AAAAJ&hl=en and > > https://github.com/chrishkchris) > > > > > > > > Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for > > Windows 10 > > > > > > > > From: Wang Wei<mailto:[email protected]> > > Sent: Sunday, 15 September 2019 2:38 PM > > To: [email protected]<mailto:[email protected]> > > Cc: [email protected]<mailto:[email protected]> > > Subject: Re: [DISCUSS] Improve code quality > > > > > > > > Hi Moaz, > > > > I agree with you. > > We did take some efforts to improve the code quality. > > [1] introduces some tools for enforcing the coding style. > > [2] introduces some tools for adding documentations. > > > > The current issue is that our contributors may have applied different > > coding styles and tools using their different editors. > > I suggest to do some tests during the CI process, e.g., running the cpplint > > and pylint. > > If all tests pass, then we merge the PR. > > > > Thanks! > > > > Best, > > Wei > > > > [1]. http://singa.apache.org/en/develop/contribute-code.html > > [2]. http://singa.apache.org/en/develop/contribute-docs.html > > > > On Sun, Sep 15, 2019 at 2:36 AM Moaz Reyad <[email protected]<mailto: > > [email protected]>> wrote: > > > > > Dear team, > > > > > > Since SINGA is going to graduate soon from the incubator, I propose to > > use > > > some tools to ensure high code quality. These tools check for known > > > problems in the code and provide a detailed report for fixing them. May > > be > > > some code came from scientific experimental projects. We need to improve > > > this code according to industry standards, so it can be used with more > > real > > > life projects. > > > > > > 1. I propose to add the code quality tools (cpplint[1], pylint[2] and > > > lgtm[3]) to SINGA contribution guideline[4], so that each developer is > > > encouraged to install and run code quality checks in his local repo and > > fix > > > any problems before creating a pull request. > > > > > > 1.A CPP Lint: running cpplint in the src directory shows 822 errors, > > while > > > running in the include directory shows 708 errors. The guidelines [4] has > > > an outdated information that instructs developers to use an old > > > non-existing file tool/cpplint.py. > > > > > > 1.B Python Lint: running pylint in python/singa shows 5.00/10 rating, > > while > > > running in python/rafiki shows 0.00/10 rating. > > > > > > 1.C LGTM :There is a Jira ticket for adding LGTM badges to the README[5], > > > so the quality of the code becomes more clear to everyone. LGTM pull > > > request automation can't be enabled in Apache repo due to infra > > > restrictions[6], but it works on personal forks of the project. Currently > > > LGTM rates both C++ and Python code in SINGA as grade D. > > > > > > 2. I propose also to give the code quality higher priority in the next > > > release since it is probably going to be the first release after > > > graduation. The team is invited to fix as much as possible from the > > current > > > code issues and to use tools that check their new code before pushing it > > to > > > SINGA. Let's try to make the lgtm grade and lint rating as high as > > > possible. > > > > > > Improving code quality is required to attract new users and developers. > > > Users will trust more the project with better code and developers will be > > > happy to contribute to it. It will also make the code review process > > easier > > > and more productive instead of wasting time in finding and fixing known > > > code problems. > > > > > > New developers (or old developers who did not contribute for a while and > > > would like to warm up) can start working on fixing lgtm and lint issues, > > > since they are usually easy and there is a clear explanation of the > > problem > > > and how to solve it. > > > > > > What do you think? > > > > > > p.s. This discussion is the first topic in a series of proposals to > > improve > > > SINGA as it will be an Apache top level project soon. The next proposal > > > will discuss improving the build and test pipeline in a separate thread > > to > > > avoid discussing too many things in one thread. > > > > > > best regards, > > > Moaz > > > > > > [1] https://pypi.org/project/cpplint/ > > > [2] https://www.pylint.org/ > > > [3] https://lgtm.com/ > > > [4] http://singa.apache.org/develop/contribute-code.html > > > [5] https://issues.apache.org/jira/projects/SINGA/issues/SINGA-484 > > > [6] https://issues.apache.org/jira/browse/INFRA-17954 > > > > > > > ________________________________ > > > > Important: This email is confidential and may be privileged. If you are > > not the intended recipient, please delete it and notify us immediately; you > > should not copy or use it for any purpose, nor disclose its contents to any > > other person. Thank you. > > >
