Hi @raff
I believe that you have enabled Codeacy to start reviewing PRs to openmrs-core (and perhaps modules). I 100% approve, but perhaps you can share the details of this (e.g. on talk, and/or on the wiki page about PRs).
Also, can we disable this one?
Issue found: Method names should not contain underscores
This triggers a ton of false positives, because our convention is to have tests with names like method_shouldDoSomething().
The project is here: https://www.codacy.com/app/openmrs/openmrs-core/dashboard. Here is a specific list of rules that can be checked: https://www.codacy.com/app/openmrs/openmrs-core/patterns/list.
It looks like that rule is disabled though, so maybe that comment is old?
Is there a reason/use case why we’re not using the GitHub plugin with our existing SonarQube installation to do this?
http://www.sonarqube.org/github-pull-request-analysis-helps-fix-the-leak/
The sonar github plugin is a new thing. I haven’t had a chance to test it out and I’m not excited to do it. The main reason is complex configuration.
I was involved in setting up Sonar for openmrs-core and I must say it wasn’t a good experience. Adding configuration is troublesome (at least for Java maven projects). It requires a special plugin and setting up credentials to Sonar on CI.
Also the full analysis takes a lot of time and resources. Our CI agents need about 20 minutes to run it for openmrs-core alone. We could only afford running it once a day, whereas ideally it should run every commit. I cannot imagine running it against all our modules. It would probably kill our CI.
We do use the preview mode for verifying each commit, but results are not persisted in the sonar database rather available as html reports under build artifacts, which are not easy to find.
The reason the comment appeared on the pull request is that @maany added the analysis of openmrs-core from a different account than our openmrs organization. He needs to disable it and use the openmrs org account instead. I sent a message to helpdesk about it.
About the Sonar plugin http://docs.sonarqube.org/display/PLUG/GitHub+Plugin
It doesn’t seem to launch a full analysis, just the GIT pull request as does Codacy.
I think it’s worth a try. In my company we use Sonar and it’s pretty easy to define new rules.
http://docs.sonarqube.org/display/DEV/Writing+Custom+Rules+using+Java
@lluismf Would you have some time to try to set this up and see how it works for us? If so, do you have the correct access to do so?
Right now our biggest concern with Codacy is that, unlike virtually all other similar services, it requires write-access to your (or someone’s) GitHub account to create comments on your behalf. If you look at other things, we use like Coveralls, Travis CI, mentionbot, etc., they all use their own application account to post comments. If we move forward with Codacy as a “standard” at a minimum we would need to create a new GitHub account like “openmrs-codacy” to minimize the risk.
I can give it a try, but I don’t have access to your systems. The access is via Telnet, Terminal Server, web ?
The webapp is at https://ci.openmrs.org/sonar/ and if you attach a public key to a new help desk case, they can get you access to the command line, if needed…
Help desk ticket created !
did you get a chance to try out the sonar github plugin?
I think its a must that we have PR code analysis be it via sonar or codacy which will help a lot in code reviews and getting rid of the technical debt. I’m willing to help out on this.
No, I haven’t. @lluismf, have you?
@teleivo, happy to assist you in whatever is needed to accomplish that. I’m in favor of codacy + a dedicated github account since I like going in the direction of cutting down on work for our infrastructure team and use cloud services whenever possible.
totally makes sense!
I dont have permissions to do it, maybe you or @dkayiwa
The github user needs project access to openmrs-core in order to add the github PR integration.
as described in the instructions:
https://support.codacy.com/hc/en-us/articles/207280219-GitHub
Can the openmrs-bot be used for it? Not sure if he has project access. But then someone who can login as the openmrs-bot would need to follow above instructions.
I think it makes sense to use the openmrs-bot account. @cintiadr, would you agree? The integration can be enabled at https://www.codacy.com/app/openmrs/openmrs-core/settings/integrations after login in as openmrs-bot (openmrs-bot may need to be added to the openmrs project at codacy first) with all checkboxes selected.
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 
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?
https://www.codacy.com/app/openmrs/openmrs-core/dashboard (the one that @pascal posted above)
https://www.codacy.com/app/infrastructure/openmrs-core/dashboard (this one is reached from github PRs)
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 
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 
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