unitsofmeasurement / unit-api

Units of Measurement API
http://unitsofmeasurement.github.io/unit-api/
Other
182 stars 42 forks source link

Get Codacy grade to A #163

Closed keilw closed 5 years ago

keilw commented 5 years ago

The Codacy grade is currently B, it should be A.

List of Codacy suggestions to be ignored/muted:

desruisseaux commented 5 years ago

We should not follow blindly Codacy (or any other tools) diagnostic. All tools have false positives, sometime in large amount. If Codacy said that a code needs to be changed, its suggestions need to be investigated on a case-by-case basis. Many times the tools is wrong.

keilw commented 5 years ago

It can be configured to skip or permanently ignore what is not appropriate for our situation. E.g. the constant prefixes.

teobais commented 5 years ago

@desruisseaux couldn't agree more, that was also my meaning here.

teobais commented 5 years ago

Taking into account the discussion above, as well as the discussion at #166 , we cannot accept each and every suggestion from Codacy (due to our standards, roadmap, etc.). That said, we need to keep a list with the issues we need to manually mute/ignore at Codacy.

Initiated that list above.

keilw commented 5 years ago

The https://app.codacy.com/project/unitsofmeasurement/unit-api/dashboard shows some issues, not sure, why https://app.codacy.com/project/unitsofmeasurement/unit-api_2/dashboard is different, if it's a matter of configuration let's try to consolidate it and delete the duplicate.

teobais commented 5 years ago

As I mentioned some time ago (in another issue or PR, I cannot recall at the moment), I have accidentally created unit-api_2 project while playing around with Codacy. However, given that I have no write rights, I think it's kinda impossible to delete it from my side. So, @keilw , I would suggest that I'm given some write rights to play a bit more, consolidate the two and delete the redundant one, so that we can move forward.

keilw commented 5 years ago

I'll try to delete it, thanks for explaining

teobais commented 5 years ago

Cool, hope we figure it out soon!

teobais commented 5 years ago

On #165 we decided to stick with prefixing method names with an uppercase letter. Hence, we should also mute related Codacy issues (an overall amount of 28 issues). Updated this issue.

keilw commented 5 years ago

Yes, that's a good idea, thanks.

teobais commented 5 years ago

By the way, did we manage to find a way to mute/ignore issues on our codacy project?

keilw commented 5 years ago

Not sure, if it needs admin rights in the Codacy profile but yes, they can be ignored either per incident or for an entire file or pattern (e.g. that "must not start with uppercase")

teobais commented 5 years ago

I think it needs; I'm currently only able to see these two projects (as I've mentioned some time ago): screenshot 2019-03-05 at 19 35 32

And even in the fore-mentioned one, I only have read rights, so, I cannot do that much.