dtcenter / MET

Model Evaluation Tools
https://dtcenter.org/community-code/model-evaluation-tools-met
Apache License 2.0
74 stars 22 forks source link

Feature #2379 sonarqube gha #2847

Closed JohnHalleyGotway closed 3 months ago

JohnHalleyGotway commented 3 months ago

Please read through this issue comment for a description of the changes and logic as well as a list of future items. Recommend proceeding with these changes for now and considering how we can refine our use of SonarQube in the future.

Once this PR is approved, I'll submit another PR to make the same changes in the main_v11.1 primarily so that the manual triggers will become available which would be very convenient for development of feature branches.

Expected Differences

Pull Request Testing

Pull Request Checklist

See the METplus Workflow for details.

JohnHalleyGotway commented 3 months ago

@hsoh-u and @jprestop these changes are now ready for your review. And they are working just how we'd like.

On my feature branch, I did intentionally introduce a SonarQube finding in the C++ code. And in this GHA run the SonarQube CXX Quality Gate check step fails just like we'd want it to! Here's a screenshot of what those new findings look like in the SonarQube server:

Screen Shot 2024-04-02 at 2 07 10 PM

In general, the SonarQube scan is compared between the code for the PR and the reference branch. If the "new code" introduces new findings, the quality gate check fails. Since I removed that intentional breakage, the final run of the SonarQube GHA workflow should pass.

The only other thing we could consider is splitting out the logic for the Python and C++ scans into separate steps. The C++ scan is WAY more complex than the Python one. If I could make a simple Python one, it'd be easier to apply it to the other Python-based METplus repos.

Should I pursue that now? Or just be happy that this setup works well?

JohnHalleyGotway commented 3 months ago

I've decided to proceed with the logic as-is. The logic for doing this in the other METplus repos will be similar but not identical. But doing both the Python and CXX in the same Docker container for MET makes a lot of sense. I'll proceed with merging this PR and will then propose the same changes for the main_v11.1 branch.

jprestop commented 3 months ago

Sounds good! Thanks!