Thanks Francis for the pointer, it works! Then I guess the right url should be: https://sonarcloud.io/project/overview?id=apache_calcite This one works for me.
Francis Chuang <[email protected]> 于2023年1月16日周一 04:46写道： > Hey Benchao, > > I also see that issue when trying to log in via > https://sonarcloud.io/project/roles?id=apache_calcite. The solution is > to log in here: https://sonarcloud.io/login, go to the Apache > organization and then go to the calcite project. > > Francis > > On 16/01/2023 12:36 am, Benchao Li wrote: > > When I open this url[1], it tells me that "You are not authorized to > access > > this page". Is this expected? > > (I'm using my Github account) > > > > [1] https://sonarcloud.io/project/roles?id=apache_calcite > > > > Alessandro Solimando <[email protected]> 于2023年1月13日周五 > 18:51写道： > > > >> Thank you Stamatis for working on this, I see no need to remove the > Sonar > >> analysis now that the quality gate is off. > >> > >> In the meantime, since all committers have write privilege in Sonar, > >> whoever is interested can help fine-tuning it to fit our needs (for > >> instance disabling rules like the code smell for TODOs if unwanted), on > a > >> voluntary basis, of course. > >> > >> Best regards, > >> Alessandro > >> > >> On Fri, 13 Jan 2023 at 11:23, Stamatis Zampetakis <[email protected]> > >> wrote: > >> > >>> I logged CALCITE-5474 [1] to disable the failures in Sonar checks > >>> allowing everything to appear as green. > >>> > >>> Sonar annotations are still going to appear under the PR after > >>> CALCITE-5474; if the intention is to remove also these indications let > >>> me know and I will log another ticket. > >>> > >>> Best, > >>> Stamatis > >>> > >>> [1] https://issues.apache.org/jira/browse/CALCITE-5474 > >>> > >>> On Fri, Jan 13, 2023 at 2:53 AM Julian Hyde <[email protected]> wrote: > >>> > >>>> Now a build 'failed' with 7 'code smells'. > >>>> > >>>> Duplicating a string literal in a test was deemed as 'critical code > >>>> smell'. For heaven's sake. > >>>> > >>>> > >>>> > >>> > >> > https://sonarcloud.io/project/issues?resolved=false&severities=CRITICAL&types=CODE_SMELL&pullRequest=2942&id=apache_calcite > >>>> > >>>> Adding '@Deprecated' without also adding a javadoc comment that > >>>> contains '@deprecated' is a 'major code smell'. (I'm guessing that if > >>>> I add a javadoc comment that only contains '@deprecated' it will tell > >>>> me that empty javadoc is a code smell.) > >>>> > >>>> And "Do not forget to remove this deprecated code someday." is an > >>>> 'info code smell'. Yeah, right. Wait until the next major version. > >>>> > >>>> I'm trying to work here. Get this *****ing robot off my back. > >>>> > >>>> Julian > >>>> > >>>> On Wed, Jan 11, 2023 at 10:41 PM Alessandro Solimando > >>>> <[email protected]> wrote: > >>>>> > >>>>> Hi everyone, > >>>>> I agree with Julian that we should be open to see what value Sonar > >>> brings > >>>>> with the current setup, but for a true accounting we need many more > >>> data > >>>>> points, two examples are just not enough. > >>>>> > >>>>> In my experience I see reviewers asking contributors to fix real > >> issues > >>>>> that Sonarlint plugin can highlight in IntelliJ even locally. > >>>>> > >>>>> So Sonar would save those reviewers time if the contributor would > >>> review > >>>>> and fix some of them autonomously. > >>>>> > >>>>> If we take Sonar as an opportunity to fix easily 80% (à la Pareto) of > >>> the > >>>>> trivial issues we generally see in contributions rather than > >>> considering > >>>>> each and every issue as blocking, we can have a positive net IMO. > >>>>> > >>>>> There are also fine tunings and exclusions to be added over time for > >>>>> accepted "issues" (like the test class under src), like Ruben was > >>>> proposing. > >>>>> > >>>>> I was the one who did the Sonar setup in Hive, I had mostly positive > >>>>> feedback by contributors who just took Sonar as an opportunity to fix > >>>> some > >>>>> bugs and improve code, the only difference is that we do not have any > >>>>> quality gate there, so the report is never marked as "failed", it's > >> at > >>>> the > >>>>> sole discretion of the contributor+reviewer to take it into account > >> or > >>>> not. > >>>>> > >>>>> I personally don't fix all possible warnings/code smells, but most of > >>>> them > >>>>> yes. Some are just fine as-is to me and they can even be considered > >>> false > >>>>> positives. > >>>>> > >>>>> Best regards, > >>>>> Alessandro > >>>>> > >>>>> > >>>>> On Wed 11 Jan 2023, 23:17 Julian Hyde, <[email protected]> > >> wrote: > >>>>> > >>>>>> The instanceof case was complicated. The code that Kevin wrote was > >>>> good, > >>>>>> and appropriate, and when Sonar blocked it Stamatis was able to > >> find > >>> an > >>>>>> alternative formulation, which existed because, by luck, the JSR > >> had > >>>>>> deprecated an exception class but not its base class. I spent 30 > >>>> minutes > >>>>>> reviewing the PR and was about to merge it, and because of Sonar’s > >>>> bump in > >>>>>> the road that time was wasted. I doubt that there has been a single > >>>> other > >>>>>> occasion when someone wrote > >>>>>> “com.example.MyClass”.equals(x.getClass().getName()) instead of “x > >>>>>> instanceof MyClass”. So far that particular check has cost us ~1 > >> hour > >>>> and > >>>>>> not saved us any time. > >>>>>> > >>>>>> I’m not saying that Sonar is net bad. I’m just saying let’s do a > >> true > >>>>>> accounting. > >>>>>> > >>>>>> > >>>>>>> On Jan 11, 2023, at 2:42 AM, Ruben Q L <[email protected]> > >> wrote: > >>>>>>> > >>>>>>> Hello, > >>>>>>> > >>>>>>> First of all, thanks Stamatis for implementing this, I think it > >> is > >>>>>>> something good for the project. > >>>>>>> In the beginning things might be a bit complicated (as always) > >> and > >>> we > >>>>>> might > >>>>>>> need some adjustments / clarifications, but I hope that in the > >> long > >>>> run > >>>>>>> we'll see this as a useful feature. > >>>>>>> > >>>>>>> Regarding the two specific issues being discussed: > >>>>>>> - If I am not mistaken, the fact that SqlOperatorTest was moved > >> out > >>>> of > >>>>>>> 'test' was a consequence of [1], see the corresponding PR [2] > >> "... > >>>> it was > >>>>>>> necessary to move several classes from the 'core' module to > >>>> 'testkit'". I > >>>>>>> don't know how simple (or how complex) a potential refactoring to > >>>> move it > >>>>>>> out of there might be. Alternatively, it seems that this is > >> rather > >>> an > >>>>>>> exceptional case, so perhaps it should qualify for an exception > >>> (e.g. > >>>>>>> everything under /testkit/* shall not be considered for > >> coverage). > >>>>>>> - Regarding the instanceof, it seems that it was indeed a valid > >>>> warning, > >>>>>>> and it has recently been fixed via [3] (see discussion on its PR > >>> [4]) > >>>>>>> > >>>>>>> Best regards, > >>>>>>> Ruben > >>>>>>> > >>>>>>> [1] https://issues.apache.org/jira/browse/CALCITE-4885 > >>>>>>> [2] https://github.com/apache/calcite/pull/2685 > >>>>>>> [3] > >>>>>>> > >>>>>> > >>>> > >>> > >> > https://github.com/apache/calcite/commit/4bebdb07c2f45a95c9a4fdf81e9bcfbdd11a15de > >>>>>>> [4] https://github.com/apache/calcite/pull/2919 > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> On Tue, Jan 10, 2023 at 3:18 PM Stamatis Zampetakis < > >>>> [email protected]> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Thanks for the feedback! > >>>>>>>> > >>>>>>>> I would like to stretch the fact that at this point it is at the > >>>>>> discretion > >>>>>>>> of the reviewer/committer to enforce or ignore the information > >>>> provided > >>>>>> by > >>>>>>>> Sonar. > >>>>>>>> > >>>>>>>> Sonar, as other similar systems, has limitations thus there are > >>>> always > >>>>>>>> going to be false positives. The rules/checks performed are > >> fully > >>>>>>>> customisable so we can enable/disable them at will. > >>>>>>>> > >>>>>>>> The two issues highlighted by Julian seem like true positives to > >>> me. > >>>>>>>> * The "New code" that was introduced recently is not covered > >>>>>> sufficiently > >>>>>>>> by tests and that's a fact. Part of the problem seems to come > >> from > >>>> the > >>>>>>>> recent modifications in SqlOperatorTest [1]. The class is > >> located > >>>> under > >>>>>>>> src/main (and not under src/test) so it is considered as a > >>>> production > >>>>>> class > >>>>>>>> and coverage checks are applied. There are ways to exclude > >> certain > >>>> paths > >>>>>>>> from coverage but I would argue that the class shouldn't be > >> there > >>>> in the > >>>>>>>> first place; we should probably log a CALCITE ticket for moving > >>> out > >>>> of > >>>>>>>> there. > >>>>>>>> * The instance of warning is something that we probably don't > >>>>>> want/cannot > >>>>>>>> fix (for the reasons mentioned in the PR) but Sonar did well to > >>>> bring > >>>>>> this > >>>>>>>> up; it is problematic to do comparisons by relying on the class > >>>> name. > >>>>>>>> > >>>>>>>> I encourage others to share their thoughts/remarks as well so > >> that > >>>> we > >>>>>>>> improve as much as possible the developer experience. > >>>>>>>> > >>>>>>>> Best, > >>>>>>>> Stamatis > >>>>>>>> > >>>>>>>> [1] > >>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >>> > >> > https://github.com/apache/calcite/blob/be7135cf1fd3d87e4036b2dd6e58d2f1251f8959/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java > >>>>>>>> > >>>>>>>> On Tue, Jan 10, 2023 at 1:38 AM Julian Hyde <[email protected]> > >>>> wrote: > >>>>>>>> > >>>>>>>>> I see two false positives so far: > >>>>>>>>> * The message on be7135cf "58.1% Coverage on New Code (is less > >>> than > >>>>>>>> 80%)" > >>>>>>>>> * The bug on PR 2942 "Use an "instanceof" comparison instead" > >>>>>>>>> > >>>>>>>>> Has Sonarcube had any true positives yet? > >>>>>>>>> > >>>>>>>>> Vladimir used to hate when I was skeptical of changes to the > >>> build > >>>>>>>>> system. But I have no tolerance for a lint system that makes > >>> extra > >>>>>>>>> work. > >>>>>>>>> > >>>>>>>>> Julian > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> On Mon, Jan 9, 2023 at 2:46 PM Francis Chuang < > >>>>>> [email protected]> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> Thanks for implementing this, Stamatis! Having code quality > >>>> metrics on > >>>>>>>>>> our repos is a huge win. > >>>>>>>>>> > >>>>>>>>>> Francis > >>>>>>>>>> > >>>>>>>>>> On 10/01/2023 4:22 am, Stamatis Zampetakis wrote: > >>>>>>>>>>> I just merged the CALCITE-5427 [1] enabling Sonarcloud > >> analysis > >>>> for > >>>>>>>>> Calcite > >>>>>>>>>>> main branch and new PRs. > >>>>>>>>>>> > >>>>>>>>>>> I have left the default Sonar quality gates active so you may > >>>> start > >>>>>>>>> seeing > >>>>>>>>>>> Sonar reporting errors in various cases. > >>>>>>>>>>> > >>>>>>>>>>> If you encounter problems or you would like things to work > >>>>>>>> differently > >>>>>>>>>>> please create JIRA tickets and ping me if you need help. > >>>>>>>>>>> > >>>>>>>>>>> Once we are happy with how things work for Calcite we can > >> also > >>>> port > >>>>>>>> the > >>>>>>>>>>> changes to Avatica. > >>>>>>>>>>> > >>>>>>>>>>> Note that all Calcite committers have administrative > >> privileges > >>>> to > >>>>>>>> the > >>>>>>>>>>> Calcite project in Sonarcloud [2]. > >>>>>>>>>>> > >>>>>>>>>>> Best, > >>>>>>>>>>> Stamatis > >>>>>>>>>>> > >>>>>>>>>>> [1] > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >>> > >> > https://github.com/apache/calcite/commit/be7135cf1fd3d87e4036b2dd6e58d2f1251f8959 > >>>>>>>>>>> [2] https://sonarcloud.io/project/roles?id=apache_calcite > >>>>>>>>>>> > >>>>>>>>>>> On Wed, Jan 4, 2023 at 5:44 PM Stamatis Zampetakis < > >>>>>>>> [email protected]> > >>>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> The integration is advancing well and I am hoping to merge > >> the > >>>> PR in > >>>>>>>>> the > >>>>>>>>>>>> coming days. > >>>>>>>>>>>> To avoid unpleasant surprises, I am planning to create a new > >>>> remote > >>>>>>>>> branch > >>>>>>>>>>>> in the main calcite repo to test some things out. I will > >>> delete > >>>> it > >>>>>>>> as > >>>>>>>>> soon > >>>>>>>>>>>> as I am done with the tests. > >>>>>>>>>>>> > >>>>>>>>>>>> Best. > >>>>>>>>>>>> Stamatis > >>>>>>>>>>>> > >>>>>>>>>>>> On Wed, Dec 28, 2022 at 2:47 PM Michael Mior < > >>> [email protected]> > >>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Thanks Stamatis! I haven't used SonarCloud before, but in > >>>> general I > >>>>>>>>> think > >>>>>>>>>>>>> such tools can be quite useful. > >>>>>>>>>>>>> > >>>>>>>>>>>>> -- > >>>>>>>>>>>>> Michael Mior > >>>>>>>>>>>>> [email protected] > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Sat, Dec 24, 2022 at 4:01 PM Stamatis Zampetakis < > >>>>>>>>> [email protected]> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Since there were no objections, I just logged INFRA-24038 > >>> [1] > >>>> and > >>>>>>>>> plan > >>>>>>>>>>>>> to > >>>>>>>>>>>>>> move this forward. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Let me know if you have questions or concerns regarding > >> the > >>>>>>>>> adoption of > >>>>>>>>>>>>>> SonarCloud. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>> Stamatis > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/INFRA-24038 > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Sat, Dec 10, 2022 at 11:32 AM Benchao Li < > >>>> [email protected] > >>>>>>>>> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thanks Stamatis for bringing this up. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I haven't used Sonar yet, but thanks for the demo[1] you > >>>>>>>> provided, > >>>>>>>>> it > >>>>>>>>>>>>>> looks > >>>>>>>>>>>>>>> really interesting. I think it's worth a try for Calcite. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> [1] https://github.com/zabetak/calcite/pull/9 > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Alessandro Solimando <[email protected]> > >>>>>>>>> 于2022年12月10日周六 > >>>>>>>>>>>>>>> 02:54写道： > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Hi all, > >>>>>>>>>>>>>>>> thanks Stamatis for the proposal and the work, I am a > >> huge > >>>> fan > >>>>>>>> of > >>>>>>>>>>>>> Sonar > >>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>> I really miss it on Calcite, so a big +1 from me on > >> this. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> In Hive so far we have received good feedback, so I hope > >>> it > >>>> will > >>>>>>>>> be > >>>>>>>>>>>>>>>> welcomed here too. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Best regards, > >>>>>>>>>>>>>>>> Alessandro > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Fri, 9 Dec 2022 at 19:02, Stamatis Zampetakis < > >>>>>>>>> [email protected] > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Hi all, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I just logged CALCITE-5427 [1] for introducing code > >>>> quality & > >>>>>>>>>>>>>> coverage > >>>>>>>>>>>>>>>>> metrics in Calcite CI. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I added some motivation and examples under the ticket > >> so > >>>> please > >>>>>>>>>>>>> have > >>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>> look > >>>>>>>>>>>>>>>>> and let me know what you think. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> If there are no objections, I will follow-up with INFRA > >>> to > >>>> set > >>>>>>>>>>>>> things > >>>>>>>>>>>>>>> up > >>>>>>>>>>>>>>>>> for the official Calcite repo. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> The integration with SonarCloud has been inspired by > >>>> HIVE-26196 > >>>>>>>>>>>>> [2] > >>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>>>> Alessandro put in place for Hive and has been very > >>> helpful > >>>> so > >>>>>>>>> far. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>> Stamatis > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/CALCITE-5427 > >>>>>>>>>>>>>>>>> [2] https://issues.apache.org/jira/browse/HIVE-26196 > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>> Benchao Li > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> > >>>> > >>> > >> > > > > > -- Best, Benchao Li
