gigabit-clowns / xmipp4-core

Core library of xmipp4
https://gigabit-clowns.github.io/xmipp4-core/
GNU General Public License v3.0
3 stars 0 forks source link

Fix Windows warnings #67

Closed oierlauzi closed 3 weeks ago

oierlauzi commented 3 weeks ago

Adopting the PIMPL idiom in more places to ensure binary compatibility. Also fixing minor warnings

MartinSalinas98 commented 3 weeks ago

@oierlauzi can you review the 11 new issues mentioned by SonarCloud?

oierlauzi commented 3 weeks ago

Dismissing the remaining SonarCloud issues (either not relevant or C++ version related).

oierlauzi commented 3 weeks ago

Warnings seem to be solved

MartinSalinas98 commented 3 weeks ago

I'd take the opportunity to review the SonaCloud rules involved in this PR. Check if they should be dismissed in this situation specifically or if they should be disabled entirely.

That way we can make sure to leave a clean PR + reduce work from the SonarCloud issue management task.

oierlauzi commented 3 weeks ago

I'd disable all "since-C++xx" rules and dismiss the other 3 specifically for this situation

MartinSalinas98 commented 3 weeks ago

I have updated the rules in SonarCloud, setting a custom "Mission critical C++11" profile, which is the same we had with a few tweaks:

oierlauzi commented 3 weeks ago

With the excuse of re-running SonarCloud, I've added a minor fix

oierlauzi commented 3 weeks ago

In general, I am not using the rule of 0. Instead I'm using the Rule of 5. Maybe there will be lots of warnings related to this.

MartinSalinas98 commented 3 weeks ago

In general, I am not using the rule of 0. Instead I'm using the Rule of 5. Maybe there will be lots of warnings related to this.

Okay, so I then created a new one extending the default profile, called "Sonar Way C++<=11", where I deactivated all the "since-c++*" rules that are above C++11.

Let's run it again and see what issues remain.

sonarcloud[bot] commented 3 weeks ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

oierlauzi commented 3 weeks ago

I'd dismiss the remaining 3 smells

oierlauzi commented 3 weeks ago

They are not useless, but I think they are out of context.