Codacy in openmrs-core PRs?

If we haven’t already, can somebody write down the steps taken to make this work for openmrs-core so we don’t have to rediscover the steps to enable this for other repositories? Eventually, we should be running Sonar and having it analyze PRs consistently across all community-supported repos.

we are now talking about using codacy for the static code analysis not sonar. the reason @raff gave is that the openmrs infrastructure is already in too much pain. this was also the reason why when I asked to use sonar for the radiology module I was told that modules cannot use sonar. I then used codacy for the radiology module and I think @raff added openmrs-core to codacy and not sure if he also uses it on another openmrs module.

We will definitely document the necessary steps in a module creation related wiki page.

No, I didn’t either.

Sounds good. I was more concerned with having consistent static code analysis of PRs in the community than which tool we use.

I’ve granted openmrs-bot account write access to openmrs-core and have enabled the integration at codacy.com. It had “PR Status” checked by default. There are also options for “PR Comment” and “PR Summary.” I enabled both of those as well.

awesome :smile: thank you!!!

codacy is already checking and commenting PRs see for example: https://github.com/openmrs/openmrs-core/pull/2010

Duplicate URLs

Does anyone know why there are two URLs for openmrs-core on codacy?

Rules/Patterns

I know that codacy supports importing rules/patterns from checkstyle.xml but I remember that when they introduced the feature it only worked automatically for new projects and you had to manually import the file in their UI for existing ones to update the rules in codacy. @raff did you import the checkstyle.xml when you set openmrs-core up on codacy? If it was you who set it up :grin:

Badge

Can someone with permission to see the settings in codacy please copy and add the badge URL into the openmrs-core readme, so it motivates us to improve our score :slight_smile:

https://support.codacy.com/hc/en-us/articles/212799365-Badges

I think it was @pascal who set it up. It wasn’t me for sure. @pascal, can you add @teleivo to be able to manage codacy for openmrs? @teleivo, what is your username there?

I’ve added the badge.

I think https://www.codacy.com/app/infrastructure/openmrs-core/dashboard is a wrong project. We should be using https://www.codacy.com/app/openmrs/openmrs-core/dashboard

1 Like

is also teleivo

interesting. When I now click on the badge I end up at

https://www.codacy.com/app/openmrs/openmrs-core/dashboard

which shows the dashboard with a banner

The GitHub integration is disabled. Please ask an administrator of this project to enable it.

so seems like there is another codacy user which is called “infrastructure” that has the integration enabled. Is the Github user openmrs-bot now codacy user “infrastructure”? @burke maybe you know this

I did enable integration for openmrs-bot as requested. Maybe it is under infrastructure instead of openmrs at codacy. I’ll check.

Ok. So, it looks like my following the steps above for openmrs-bot created the /infrastructure/ path on codacy. I didn’t know @pascal had already set up an account (org?) on codacy. I did check for any shared infrastructure credentials for codacy.com first, but didn’t see any, so just authenticated as openmrs-bot via GitHub.

We can certainly (hopefully) delete the “infrastructure” account set up for openmrs-bot and move it under /openmrs/, but it looks like @pascal will need to do that, since I don’t know how the /openmrs/ codacy account is set up (I can’t find shared credentials via LastPass or any mention of codacy on the IT wiki).

Codeacy is improperly flagging unit test names with

Issue found: Method names should not contain underscores

Example: https://github.com/openmrs/openmrs-core/pull/2012#pullrequestreview-23612327

(If that was fixed sometime since my first post on this thread in April 2016, but if so, it has regressed.)

  1. I dont know if codacy supports the checkstyle suppressions where we can exclude this rule only for test classes (see). I am asking their support.

  2. The way I solved it for the radiology module, which is not ideal, is excluding the test directorys. This can be done in the codacy UI.

If I am given permission to change settings on codacy I can for now apply the 2. option.

Sorry guys, I’ve just noticed I’m actually added to openmrs project admins on codacy. I’ve added burke (by your openmrs e-mail) to the project. @burke, you should be able to set up the integration now with openmrs-bot. Please disable the one in the infrastructure project.

I just wanted to point out, that the warning is coming from PMD and not checkstyle. It should be suppressible by adding a ruleset.xml file, which codacy should find. See [0] & [1]

its a bit more complicated.

  1. we are using checkstyle, the rules are in the repo which we need to load into codacy (as codacy has an autoload feature but it only works when the file is in the root of the repo). also I see we have different rules for webapp which I dont think works in codacy.

  2. the issue you are having is because codacy doesnt take into account our checkstyle suppression rules which are in a separate file. The workaround (which is not ideal) I just applied until I know of some other solution is that I added the api/src/test and web/src/test folders in the codacy UI to the ignored folders so this will help with your and other PRs.

  3. currently we are having a bit of a mixup. @burke can you please remove the infrastructure user/account you created for the openmrs-bot (since this one currently has the github PR integration and uses other rules; and does not ignore the test directories).

@raff added us both to the openmrs account on codacy as admins so you should then be able to add the integration on the correct codacy account here https://www.codacy.com/app/openmrs/openmrs-core/dashboard then the PRs should be fine again

Why not simply drop Codacy and keep using Sonar?

its been already mentioned in the thread but in summary:

  • we need github PR integration to drastically improve code quality of openmrs core and to then keep quality
  • we should have consistent code style/rules across modules
  • openmrs infrastructure is already in pain so even if we use the sonar github plugin for openmrs core, modules will not be able to use the same setup

I dont mind which tool we use but we need github PR integration

I doubt that Sonar is the culprit of the infrastructure problems (at least JIRA was crashing because out of memory problems, and RAM is not expensive ).

Anyway, SonarQube is also available in the cloud, free for OS projects. And IMHO the UI is better than Codacy’s

I personally don’t mind if developers prefer Sonar or Codacy, or even both.

But in all fairness, sonar is not causing any big harm to our infrastructure, indeed. If you tell me that we can have the same software for free in the cloud, with all the features we need, I’d totally prefer that :smiley:

good to know, thanks @lluismf

To summarize

I think we agree on:

  • OpenMRS community would be better of not hosting static code analysis itself
  • we should add github PR integration for static code analysis on openmrs-core (later on other modules as well to establish a common style)

What we dont yet fully agree:

There is more work needed to establish the github PR integration on https://sonarqube.com as shown https://docs.sonarqube.org/display/PLUG/GitHub+Plugin at the end where they refer to https://github.com/SonarSource/sonarqube/blob/master/travis.sh thats why I was leaning towards codacy