Errata corrige: the right PR link is the following https://github.com/apache/hive/pull/3254
Best regards, Alessandro On Mon, 8 Aug 2022 at 10:04, Alessandro Solimando < alessandro.solima...@gmail.com> wrote: > Hi community, > in the context of HIVE-26196 > <https://issues.apache.org/jira/browse/HIVE-26196> we started considering > the adoption of SonarCloud <https://sonarcloud.io/features> analysis for > Apache Hive to promote data-driven code quality improvements and to allow > reviewers to focus on the conceptual part of the changes by helping them > spot trivial code smells, security issues and bugs. > > SonarCloud has already been adopted and integrated into a few top Apache > projects like DolphinScheduler <https://dolphinscheduler.apache.org/> and > Apache > Jackrabbit FileVault <https://jackrabbit.apache.org/filevault/>. > > For those who don't know, Sonar is a code analysis tool, the initial > adoption would aim at tracking code quality for the master branch, and > making the PRs' review process easier, by allowing to compare which > code/security issues a PR solved/introduced with respect to the main branch. > > We already have a Hive-dedicated project under the Apache foundation's > SonarCloud account: https://sonarcloud.io/project/overview?id=apache_hive. > > In what follows I will highlight the main points of interest: > > 1) sonar adoption scope: > For the time being a descriptive approach (just show the analysis and > associated metrics) could be adopted, delaying a prescriptive one (i.e., > quality gates based on the metrics for PRs' mergeability) to a later time > where we have tested SonarCloud for long enough to judge that it could be a > sensible move. > > 2) false positives: > Sonar suffers from false positives, but they can be marked as such from > the web UI: (source https://docs.sonarqube.org/latest/faq/#header-1) > > How do I get rid of issues that are False-Positives? >> False-Positive and Won't Fix >> You can mark individual issues False Positive or Won't Fix through the >> issues interface. If you're using PR analysis provided by the Developer >> Edition, issues marked False Positive or Won't Fix will retain that status >> after merge. This is the preferred approach. > > >> //NOSONAR >> For most languages, SonarQube supports the use of the generic mechanism: >> //NOSONAR at the end of the line of the issue. This will suppress all >> issues - now and in the future - that might be raised on the line. > > > For the time being, I think that marking false positives via the UI is > more convenient than using "//NOSONAR", but this can be discussed further. > > 3) test code coverage: > > Due to the specific structure of the ptest infra (split execution and > other peculiarities), we are not yet supporting test code coverage, this > can be added at a later stage, in the meantime all the code quality and > security metrics are available. > > 4) what will be analyzed: > > the master branch and each open PR > > 5) integration with github: > > SonarCloud integrates with GitHub in two ways, the first one is an > additional item in the list of checks (where you have the spell checking, > CI result etc.) that will just say Passed/Not Passed and provide a link for > all the details, the second is a "summary" comment under the PR > highlighting the main info (you can see an example here > <https://github.com/apache/hive/pull/3254#issuecomment-1206641629>). > > The second integration can be disabled if we consider that the first one > is enough, and that if we want to dig more we can open the associated link > for the full analysis in SonarCloud. > > 6) analysis runtime: > > In CI the full analysis takes around 30 minutes, but this step is executed > in parallel with the test split tasks and won't add to the total runtime. > For PRs SonarCloud detects unchanged files and avoids analysing them, so > the runtime there is expected to be lower. > > I'd like to hear your thoughts on this, and I am looking for reviewers for > the PR https://github.com/apache/hive/pull/3339. > > Best regards, > Alessandro >
