Codacy annoyances/changes

55 views
Skip to first unread message

Matthew Jones

unread,
Aug 24, 2018, 9:27:58 AM8/24/18
to Sakai Development
Hi Sakai Dev,

I'm sure some of you have been annoyed with all of the Codacy comments inline in Gthub issues. I've also noticed many of the comments are just deleted from the issues on Github. I added Codacy on some new projects I started working on locally and generally love the feedback but made the following changes to Sakai's Github integration. [1]

- Disabled the individual inline Codacy comments on Github issues
- Enabled adding a summary of found issues to the Github after reviewed (haven't seen this in action yet, I'm but the screenshot looks good)
- The status link is still enabled and should not block the merge if issues are found

Use these new links to reduce the number of issues introduced.

In addition 
- I changed value for "Javascript Cyclomatic complexity" up to 10 (from 4) [1] as that's still reported to be in the "easy to maintain" category. Many of the ones I saw flagged were 5 and 6.
- I removed the check for "Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes." which had over 3000 occurrences and really doesn't seem like an issue to me.
- I disabled SQLlint as I noticed it only checks Postgres parser so will report lots of false positives. Hopefully all this SQL that's in there is 'tried and true' ;)

Thanks!

Steve Swinsburg

unread,
Aug 25, 2018, 6:59:18 AM8/25/18
to Matthew Jones, sakai-dev
Codacy will only annoy you if you have something wrong with your code and you don't address it.

The aim is to organically get the codebase in better shape.

Many of the codestyle fixes could be resolved if everyone used a decent linter/code formatter preferences in Eclipse. I am happy to share the one I use. It automatically resolves many issues that Codacy would complain about.

Dropping the inline commenting to a summary is fine if you can still go to the actual issue. The sonar plugin doesn't do this very well so I'm hoping Codacy is better?

I think a lot of those deletes are just Codacy cleaning up once people resolve issues? Or are people actively ignoring the Codacy comments?

Cheers
Steve



--
You received this message because you are subscribed to the Google Groups "Sakai Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sakai-dev+...@apereo.org.
To post to this group, send email to saka...@apereo.org.
Visit this group at https://groups.google.com/a/apereo.org/group/sakai-dev/.

Matthew Jones

unread,
Aug 25, 2018, 8:42:39 AM8/25/18
to Steve Swinsburg, dev sakai
Well this was the issue that prompted the change (commented about in the slack channel)

There were dozens of issues that indicate comment from codacy-bot deleted by steveswinsburg. Was that you (Steve) that deleted them or Codacy deleting them with your name since you integrated it? I can't tell if these were fixed or not but many times issues like this aren't from new original work but just old code that someone has to refactor that had previous issues in it. Some of these issue are a result of misconfiguration (like the javascript is not defined) which can take some time to setup. Others on various style changes are just suggestions and things that really don't need to be fixed as they match the style that exists in the files. 

The "Codacy summary" is seen in 
https://github.com/sakaiproject/sakai/pull/5915

I personally think this summary is just as good and allows someone to click through to see the issues and fix them or comment more about them if they consider them an issue. It will never be 100%, Sakai may not even get to 10% or 5% (Currently at 14%) unless some checks were just disabled.

Anyway, if the more people want Codacy comments back on, it's easy enough for one of us to just click the checkbox!

Charles Severance

unread,
Aug 25, 2018, 12:37:37 PM8/25/18
to Steve Swinsburg, Matthew Jones, dev sakai
Steve, I loved the Codacy review and carefully fixed all of them until I got tired to wait 30 minutes because it seems to only run when I send in a PR.

If I could run it at the command line I would fix things in my patches before I sent in a pull request. 

I don't want to run it at the command line and find 10,000 problems across my entire code base - I want to see issues with my patches.

After about 3 PR's I stopped trying to make it happy because it added so much turn around time.

So if I can find a way to use it pre-PR I would love it.

Thanks.

/Chuck

On Sat, Aug 25, 2018 at 6:59 AM Steve Swinsburg <steve.s...@gmail.com> wrote:

Steve Swinsburg

unread,
Aug 26, 2018, 5:32:59 AM8/26/18
to Matthew Jones, sakai-dev
I've noticed those deletes, I think it's just that it was integrated by me - no deletes are being done by me! I'll have to look into it to see what that is all about.

Yeah a lot of the issues are for old code but in most cases it's really helpful to fix other issues there as well, so improve the overall quality.

In terms of the summary I think we can try it out. It doesn't actually show the issues which is what this whole thing is for, so if people are ignoring it then it might as well be removed, which would be a shame. But if it is still working with the click through then it could be ok.

Regardless I think a threshold should be enabled to block PR merging when major issues exist. It's the only way to get people to fix problems.

In sonar there is a way to mark problems as won't fix or false positives. We can also tweak the rules.

Cheers
Steve

Steve Swinsburg

unread,
Aug 26, 2018, 5:34:21 AM8/26/18
to Severance Charles, Matthew Jones, sakai-dev
I'll look into this. Even if we used the eclipse codestyle plugin I use then 90% of the issues are identified immediately.

Regards
Steve

Charles Severance

unread,
Aug 26, 2018, 9:54:13 AM8/26/18
to Steve Swinsburg, Matthew Jones, dev sakai
Is there a way I can put it in my fork?  That way I could clean it up before I send the PR?

/Chuck
Reply all
Reply to author
Forward
0 new messages