SonarQubeCommunity / sonar-erlang

12 stars 23 forks source link

Erlang plugin does not support for SQ 6.7 LTS #25

Closed anandkrv closed 6 years ago

anandkrv commented 6 years ago

Can we have Erlang support for SQ 6.7 LTS?

kalidasya commented 6 years ago

@anandkrv I started the update, see sonarqube56 (well, I guess we can just call it sonarube67) but do not think I will have time. Maybe with some community support? :)

yuniers commented 6 years ago

@kalidasya What kind of changes must be done by community for update it for SQ 6.7 LTS?

kalidasya commented 6 years ago

@yuniers its a bit hard to predict, you need to change all sonar dependency to point to the lts versions in the parent pom, then see what is broken or deprecated, afaik the Issue interface is changed and some other so those needs to be changed then its compatible again. sonarsource has a confluence page where they list the non backward compatible changes in the api: https://docs.sonarqube.org/display/DEV/API+Changes but as you see they are more like hints you still need to figure out what is the intended usage, then I usually turn to an existing plugin to see how it is done.

yuniers commented 6 years ago

@kalidasya yes, is very complicated say what must be changed.

kelsaka commented 6 years ago

@kalidasya Is there a chance you will have a look into SQ 6.7 compatibility?

kalidasya commented 6 years ago

@kelsaka to be honest not much, as you saw I tried to fix it for 5.6 already and that was not finished so I tried 6.7 months ago but as its not straightforward I doubt I will ever find the time. maybe someone from SonarSource can help.

marcphilipp commented 6 years ago

We've started with making the necessary changes in a fork, starting with the sonarqube56 branch: https://github.com/SonarQubeCommunity/sonar-erlang/compare/sonarqube56...marcphilipp:sonarqube67

It compiles and all the tests run. However, when running a local analysis, only the coverage seems to be reported correctly. Neither issues, duplication checks, nor highlighting works. Since we have no previous experience writing a SonarQube plugin, any help on what might be causing this or how to properly debug it would be highly appreciated.

@kalidasya Any ideas?

kalidasya commented 6 years ago

@marcphilipp thats great news, thanks for your effort! in 6.7 if I remember correctly they separated the database interactions so I am pretty sure the whole saving metrics and issues is different, it can be that something simple is missing like a save call what was not required before. If the tests are passing then its weird, have you get rid of all deprecation warnings? usually this is how I start, so I know I am using the new and working interfaces

kalidasya commented 6 years ago

@marcphilipp as I see you still use a lot of deprecated api, I think that is the problem, I would start with getting rid of them (at least in one area to confirm), the API changes page is useful so are the other plugin implementations. the unit tests will not necessary catch these things as they are mocking the server interface so things are being called but if it will work at the end its not guaranteed

marcphilipp commented 6 years ago

We only had a limited amount of time and were hoping we could live with the deprecation warnings for now. However, looks like we need to resolve them to make it work. Thanks for the feedback!

kalidasya commented 6 years ago

@marcphilipp honestly I am not 100% sure if thats the cause so I would test it first (syntax highlighter is small for example)

marcphilipp commented 6 years ago

I've updated the branch in my fork. I've fixed checks (see 5030ac247f6170a4a7159416e7ab4da7a8a4513e) and the highlighter (see d456ba59ca650827bd7648c9b2bb305b02507082). I've also made a change related to CPD (cf. 09ad53720acec55efd8bdfe09bc82b663d37c16e) that fixed the warnings logged at runtime but I wasn't able to trigger it by duplicating and analyzing code.

I've only just now noticed that the sonarqube56 branch and the master branch have diverged with conflicting changes to DialyzerReportParser.

@kalidasya Since you made the changes to the branch and merged the PR to master, could you please attempt to merge master into sonarqube56?

kalidasya commented 6 years ago

@marcphilipp will check

kalidasya commented 6 years ago

@marcphilipp should be there, not fully tested but merge conflicts resolved

kalidasya commented 6 years ago

@marcphilipp its released, it will not appear in the update center though as I requested to get it removed. people need to get the jar from the release

marcphilipp commented 6 years ago

Thanks! 🙂