Community
OpenEnergyMonitor

OpenEnergyMonitor Community

Emoncms Codacy automated code reviewer

Tags: #<Tag:0x00007fc9c42ce850> #<Tag:0x00007fc9c42ce788> #<Tag:0x00007fc9c42ce6c0>

In a previous topic I talked about GitHub integration tools to improve the quality and confidence of the generated code.

Recently I have tested a very interesting tool for security and quality of the code generated. I am talking about Codacy. The pricing for open source projects is free.

I have tested EmonCMS project and dashboard plugin and they have a lot of potential errors (error prone). The project certification is C in both cases:

EmonCMS: https://www.codacy.com/app/RDCH106/emoncms/dashboard
Codacy Badge

Dashboard: https://www.codacy.com/app/RDCH106/dashboard/dashboard
Codacy Badge

We must fix the potential errors and improve the code certification.

What do you think about it?

1 Like

I don’t see any reason why not. I have enabled codacy on the emoncms/emoncms repo and added the badge to the github Readme. PM me your email and I’ll add you as a developer. I’ve not used Codacy before.

https://www.codacy.com/app/emoncms/emoncms/dashboard


On the topic of continuous testing, I would like to setup TravisCI to perform a range of tests on Emoncms e.g. test the API by posting some data then retrieving it. In the future, we could expand to stress testing and report performance implications of

About tests it could be possible to add a very simple python tool to test the API. The python tool could be other repository and add it as submodule for EmonCMS.

To test the graphical behaviour, the best way is using a framework like Selenium. I will investigate it.

Having looked at the Codacy output - there are two issues, and both are fixable, one we have some PHP scripts that exit on failure, well yeah we should be able to fix those in one of a few possible ways, and the others are CSS issues that again should be fixable.

Neither are particularly worrysome.

EDIT: Colour me stupid - there are many more pages of stuff… Ahh well :slight_smile:

1 Like

There are errors, but a lot of the issues reported by Codacy are superficial & debatable…
…for example if all of the camelcase issues were sorted, we’d probably reduce the outstanding issues by 50% :wink:

Paul

All the reports are categorized. For example code style errors are not problematic for the working but they are problematic to the new people who want contribute.

The unused code is a problem always but it is not evident. The code unused is there and it can be used in attack and can be include errors.

The category of potencial errors is the most serious because they can be error using the sections of code or if the language change the behaviour for the code section because it is not a good practice.

Each marked point is important in its own way.

How do you pull in different branches to analyze?

I’m in the ‘Codacy emoncms organisation’, but I can only see the master branch of the ‘Dashboard’ repo. It would be useful to look at the other 2 ‘dashboard’ project branches before being eventually merged to master.

I see Codacy has listed the ‘stable’ branch, as well as master for the emoncms core repo, so it can be done.

Paul

I think that we must focus the attention in error prone certification:

https://www.codacy.com/app/emoncms/emoncms/goals?bid=3864434

There are error related with external Libs. First we must solve the errors of code of the project that correspond to our codification (excluded external libs):

https://www.codacy.com/app/emoncms/emoncms/issues?bid=3864434&filters=W3siaWQiOiJMYW5ndWFnZSIsInZhbHVlcyI6W251bGxdfSx7ImlkIjoiQ2F0ZWdvcnkiLCJ2YWx1ZXMiOltudWxsXX0seyJpZCI6IkxldmVsIiwidmFsdWVzIjpbIkVycm9yIl19LHsiaWQiOiJQYXR0ZXJuIiwidmFsdWVzIjpbbnVsbF19LHsiaWQiOiJBdXRob3IiLCJ2YWx1ZXMiOltudWxsXX1d

I have mentioned the use of submodules in a PR. If we do that, We can achive less error produced by the external libs because they are not present. In this case we must have blind trust in the external libraries, but the maintenance and update of external libs is more friendly. Always can do a fork of the library and check the certification of the library in Codacy. If we dont update the external libs the project will have the same errors in external libs.

The developer team (where you are) was without projects assignation. Now is done!
Cceck again.

1 Like

I think the error prone certification goal needs to be for the master branch and not stable.
If the code is improved in the master branch, and proves successful, it would be merged across to stable.

Paul

You are right!!

https://www.codacy.com/app/emoncms/emoncms/goals?bid=3864432

Done!

Another topic related.

We must try to merge PR and push without Codacy errors.

Many errors are related with bad practices when we are coding.

The double checking should be passed with success.

Just looked at your last PR in Codacy, and which in addition to adding a new feature, fixes 2 existing issues;
'_Tr' is not defined'
What have you done to fix it!! I can’t see a declaration added…

Paul

This is a very nice and usefull addition that will highlight some issues.
But…
Please don’t start a ‘clean all warnings journey’ without thinking if the effort worth it.
They are exactly that, warnings and the threshold is configurable.

The two errors persist in the code. Surely when I update de definition the errors disappear but when Codacy check the new definition the errors are there.

It is a strange thing! The real result is that there are two errors.

And as says @nchaveiro Codacy is an orientation.