-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature 2673 sonarqube beta6 vector dictionary #2994
Feature 2673 sonarqube beta6 vector dictionary #2994
Conversation
…qube_beta6_vector_dictionary
…qube_beta6_vector_dictionary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this testing workflow run failed. Please fix those failures and re-request my review.
I rolled back the changes (pointer to vector) at src/tools/core/mode/mode_exec.cc to avoid introducing a new bug. Looks like unittest is not done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve of these changes. I see that the GHA testing workflow flagged no differences and that the total number of SonarQube findings are reduced from 18,476 to 18,331.
I do note that the bug in src/libcode/vx_python3_utils/wchar_argv.cc
remains, and we'll need to fix that prior to the MET-12.0.0 release in December, 2024. But these changes do reduce the overall number of code smells as intended.
Please review again.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve of these changes.
I see that the SonarQube code smells are further reduced to 18,323.
Expected Differences
Reduced code smells on python code
Used vector instead of new statements.
One bug was identified as a new bug at
src/tools/core/mode/mode_exec.cc
. I will take a lookDo these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:
Pull Request Testing
Unittest with the same outputs
N/A
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
Do these changes include sufficient testing updates? [No]
Will this PR result in changes to the MET test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update Truth METplus issue to describe them.
Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
One bug was identified as a new bug at
src/tools/core/mode/mode_exec.cc
.Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s) and Development issue
Select: Milestone as the version that will include these changes
Select: Coordinated METplus-X.Y Support project for bugfix releases or MET-X.Y.Z Development project for official releases