But it seems to me that the most of the rules mentioned there are valid and adhered only by the openmrs-core. So for example checkstyle plugin is not included in most of the modules. And I’ve also found formatting to be inconsistent. Tabs, and spaces are mixed up even within the same file.
So wouldn’t it be handy if we enforce those quality checks into their poms itself as done by the openmrs-core?
Or may be we could have a master parent pom which other modules can inherit, for common things such as repositories, plugins and distribution management?
If required, I could try implementing and testing it in the
webservices.rest
module since I’ll be working on it this summer.
,[quote=“gayanw, post:1, topic:11386”]
Or may be we could have a master parent pom which other modules can inherit, for common things such as repositories, plugins and distribution management?
[/quote]
-> Sounds like a resonably good idea to me, what do you think @dkayiwa@pascal@darius
As for now, the general idea is to simply follow the existing conventions followed in the module itself. If you see tabs and spaces mixed up, then here’s what you can do to correct all formatting in one go.
This is what I had done for the cohort module I worked on last summer, assuming you are using Intellij, there should be a ‘Java Settings’ options where you can set whether braces after if conditions are compulsory, whether to use spaces or tabs, the indentation size and all of that. I suggest you set those values to what matches closest to openmrs-core and then just click on the right click the module and select Reformat Module. That should enforce reasonable consistency in the module you are working on. Make these changes on a branch and issue a PR after consulting a /dev/5 about the changes (Just create a ticket?), and continue with your work as usual.
So if we are to use openmrs-core’s checkstyle.xml remotely by any other module or project, their is the need to make some minor changes to the checkstyle file. I raised an issue on this. May be @teleivo can have a look.
Why codacy and not maven checkstyle plugin
The choice for using codacy is so that devs and also reviewers get an immediate feedback on the code quality of a PR. We do not use the checkstyle maven plugin in our build because the errors it finds will only show in the build log which a lot of the devs creating PRs are not checking.
Codacy configuration
With codacy you can either place the config files in the root directory of your repository or configure codacy manually in their UI. We chose the first way since we want to everyone to be able to see, change and reuse the style config we agree on.
I cannot configure a path to a checkstyle suppression file in codacy. Therefore I set the SuppressionFilter in checkstyle.xml hoping that codacy would pick it up. I am not sure it actually does that. Can you please figure that out? Because if codacy doesnt we can remove this section and use your suggestion in other modules.
Codacy is not being used by all the modules right? I think, it would still be useful to have maven integrated into the pom without actually binding it to a Maven lifecycle phase, so one can simply run a checkstyle:check before pushing their code.
I tried adding openmrs-core repo in my codacy, but it didn’t even pickup the checkstyle configuration file. So I’m not sure if SuppressionFilter is read by codacy or not. However a workaround is to use the url instead of the file name so:
its true that it is useful to run checkstyle locally, but my experience is almost no one does it (same with formatting) Therefore I am for using codacy since it automates that and forces people to see the quality of their contribution.
Interesting workaround the issue I see with the web links to checkstyle files is that you will always need an internet connection to run checkstyle, even if you have all the maven dependencies you need to build/run the project locally.
Checkstyle plugin not following remote suppression files actually is a bug which was fixed in later versions. So changes to checkstyle.xml is no longer necessary which is a good thing. Sorry I forgot to update the thread.
Therefore I am for using codacy since it automates that and forces people to see the quality of their contribution.
the issue I see with the web links to checkstyle files is that you will always need an internet connection to run checkstyle, even if you have all the maven dependencies you need to build/run the project locally.