Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static/Dynamic code analysis tools #30

Closed
fredjeronimo opened this issue Nov 1, 2018 · 11 comments
Closed

Static/Dynamic code analysis tools #30

fredjeronimo opened this issue Nov 1, 2018 · 11 comments

Comments

@fredjeronimo
Copy link
Contributor

In Prompt:

I've used quite a few other tools in the past such as Pc-Lint, NDepend, FxCop/StyleCop and Coverity. The latter is by far the most powerful one but it's quite expensive...

@smobs
Copy link
Contributor

smobs commented Nov 1, 2018

We use sonarqube on masketeers. Feels like RG has adopted this

@ChrisHurley
Copy link
Contributor

I haven't had much value out of SonarQube but I think it's been tried fairly widely, yeah.

@nyctef
Copy link
Contributor

nyctef commented Nov 1, 2018

+1 for not sure exactly how much value sonarqube provides - we have it on our PRs and it seems to occasionally pop up with some useful suggestions (and seems to be more intelligent / better at finding actual bugs than R# or compiler warnings) but we still spend a nonzero amount of time managing false positives and stuff we just don't care about.

I'd be interested in potentially exploring some of the alternatives @fredjeronimo mentioned to see if they do any better

@ChrisLambrou
Copy link
Contributor

We use code inspections in R# and TeamCity in The Spiders. It's kind of annoying at times, but it's nonetheless keeping us honest for a good handful of basic code quality issues.

@garethbragg
Copy link
Contributor

Considering how widely it's used, I do think Sonarqube should be in Adopt.

Quite happy for any alternatives to be in Explore.

@ChrisLambrou
Copy link
Contributor

Ooops, I posted the following in #56, but it's probably more pertinent here:

@fffej, @garethbragg: FWIW, we've pretty much decided to ditch SonarQube analysis on SQL Data Catalog. It's just flaky as hell, causing many failed builds that are fixed simply by rerunning them, and it's never given us any value beyond the code inspections feature in R# and TeamCity, which is way more reliable.

Regarding the flakiness, look here: https://buildserver.red-gate.com/viewType.html?buildTypeId=CustomerDelight_SqlEstateManager_Web_2SonarqubeAnalysis
Every single failure comes with the error message "The SonarQube MSBuild integration failed: SonarQube was unable to collect the required information about your projects." I can't remember a time when it failed because of a legitimate code issue.

I'm a bit surprised this got promoted to Adopt to quickly given its luke-warm reception in this thread, and plenty of evidence that maintaining it is an annoyance and a distraction.

@nyctef
Copy link
Contributor

nyctef commented Nov 29, 2018

Regarding the flakiness, look here: https://buildserver.red-gate.com/viewType.html?buildTypeId=CustomerDelight_SqlEstateManager_Web_2SonarqubeAnalysis

This appears to be the powershell + stderr + rg-buildphy02 problem that's been popping up recently: (eg [1] and [2]) - it's really annoying but possible to work around so that the build doesn't fail because of random warnings. Sonarqube is likely not doing anything wrong here.

I'm a bit surprised this got promoted to Adopt to quickly given its luke-warm reception in this thread, and plenty of evidence that maintaining it is an annoyance and a distraction.

Well, it got promoted to Adopt since it's currently widely adopted, and we're currently just trying to model the existing state of the world here. We can definitely look into exploring alternatives as well (I know several teams use R# inspections for a similar thing and I think people have looked into StyleCop/FxCop/etc).

Personally I think the sonarqube inspections are slightly better / more useful than other tools I've used, but the build issues are annoying, and it's only going to provide marginal benefit if you're using a similar tool already. Maybe it's just a case of picking one static analysis tool (per team) and sticking with it?

@ChrisLambrou
Copy link
Contributor

ChrisLambrou commented Nov 29, 2018

If SonarQube requires a special workaround that no other tool seems to require, then maybe we should codify that workaround into a RedGate.Build cmdlet? e.g. invoke SonarQube analysis with -ErrorAction Continue, but separately capture any stderr output and subsequently raise an error directly in PowerShell if necessary? Would something like that solve the stability problems, or are they more wide reaching?

@ChrisLambrou
Copy link
Contributor

Plus, adding support for SonarQube in RedGate.Build would mean I can get rid of stuff like this that I just don't want to have to maintain in our project's repo.

@nyctef
Copy link
Contributor

nyctef commented Nov 29, 2018

If SonarQube requires a special workaround that no other tool seems to require, then maybe we should codify that workaround into a RedGate.Build cmdlet?

Git actually has the same problem with powershell and stderr as well (although it turns out that git has a workaround built-in through an environment variable you can set)

I agree we should probably pull something together in RedGate.Build for running sonarqube, though, for the reasons you said - it doesn't look like our various copies of sonarqube.tasks.ps1 have diverged much.

@ChrisLambrou
Copy link
Contributor

I agree we should probably pull something together in RedGate.Build for running sonarqube, though, for the reasons you said - it doesn't look like our various copies of sonarqube.tasks.ps1 have diverged much.
👍

They haven't diverged "much", which means they might have a little bit. That's grounds enough to pull it into RedGate.Build. I think that's my 10% time job for tomorrow! 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants