Skip to content

Conversation

jhaigh0
Copy link
Contributor

@jhaigh0 jhaigh0 commented Mar 17, 2025

Description of work

  • A couple std::string.starts_with warnings
  • A warning about searching a set before inserting
  • Some useStlAlg warnings

There is no associated issue.

To test:

  • Code check

Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review

  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Do the release notes conform to the release notes guide?

Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

@jhaigh0 jhaigh0 added Maintenance Unassigned issues to be addressed in the next maintenance period. ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions labels Mar 17, 2025
@jhaigh0 jhaigh0 added this to the Release 6.13 milestone Mar 17, 2025
@jhaigh0 jhaigh0 moved this to Awaiting Approval in ISIS Core Mar 17, 2025
@jhaigh0 jhaigh0 force-pushed the some_stl_cppchecks branch from 8b034a2 to 82dec3f Compare March 17, 2025 11:16
@jhaigh0 jhaigh0 force-pushed the some_stl_cppchecks branch from 82dec3f to 99b8f4f Compare March 18, 2025 10:49
@jclarkeSTFC jclarkeSTFC self-assigned this Mar 27, 2025
@jhaigh0 jhaigh0 requested a review from jclarkeSTFC March 28, 2025 13:48
@github-project-automation github-project-automation bot moved this from Awaiting Approval to Awaiting Merge in ISIS Core Apr 1, 2025
@SilkeSchomann SilkeSchomann self-assigned this Apr 1, 2025
@SilkeSchomann SilkeSchomann merged commit 8cc51f7 into mantidproject:main Apr 1, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done in ISIS Core Apr 1, 2025
peterfpeterson pushed a commit to peterfpeterson/mantid that referenced this pull request Apr 1, 2025
* some stl related cppchecks

* additional ccpcheck fixes from line numbers moving

* remove fix that I think is causing the test failure

* use std optional to avoid uninitialised warning
peterfpeterson added a commit that referenced this pull request Apr 1, 2025
* Cppcheck suppressions set 56 (#38744)

* Cppcheck suppressions set 56

* Remove lines from suppressions and other fixes

Py_RETURN_NONE renamed to Py_NONE and return removed (now to be done manually) as not used anywhere else and was confusing cppcheck with missing return.

Had to make lambda function wrapper if wanted to return const pointer in AlgorithmProperty.cpp

Co-authored-by: Mohamed Almaki <MohamedAlmaki@users.noreply.github.com>

* Not use macro to return

* Replace std::function pointer with a C style function pointer

* Update Framework/PythonInterface/mantid/api/src/Exports/IPeakFunction.cpp

Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>

* Update Framework/PythonInterface/mantid/api/src/Exports/IFunction1D.cpp

Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update Framework/PythonInterface/mantid/api/src/Exports/IFunction1D.cpp

Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>

* Update Framework/PythonInterface/mantid/api/src/Exports/IPeakFunction.cpp

Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>

* Revert constParameterCallback suppression fix

---------

Co-authored-by: Mohamed Almaki <MohamedAlmaki@users.noreply.github.com>
Co-authored-by: Mohammed Almakki <mohammed.almakki@stfc.ac.uk>
Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix some of the simpler knownConditionTrueFalse warnings (#39004)

* fix some knownConditionTrueFalse warnings

* fix cascading ccpcheck warnings

* Review suggestions

Co-authored-by: James Clarke <james.clarke@stfc.ac.uk>

---------

Co-authored-by: James Clarke <james.clarke@stfc.ac.uk>

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pre-commit/mirrors-clang-format: v19.1.7 → v20.1.0](pre-commit/mirrors-clang-format@v19.1.7...v20.1.0)
- [github.com/astral-sh/ruff-pre-commit: v0.9.10 → v0.11.2](astral-sh/ruff-pre-commit@v0.9.10...v0.11.2)

remove unnecessary brackets

Update cppcheck suppression line number

* Pin conda=25.1.1 to avoid CMake FindPython error in Windows packaging (#39119)

* A variety of Cppcheck fixes (#39115)

* Delete unnecessary virtual destructors.

* Fixes for ImportMDHistoWorkspaceBase

The member variable nbins is unused, and was shadowed inside a method.
Also made some variables in that method const.

* AlgorithmDialog missing const in a few places

* Fix some return by reference in FitPropertyBrowser

* Fix several passedByValue Cppcheck warnings

* Further Cppcheck fixes

* Some stl related cppcheck fixes (#39072)

* some stl related cppchecks

* additional ccpcheck fixes from line numbers moving

* remove fix that I think is causing the test failure

* use std optional to avoid uninitialised warning

* Pin pydantic to fix failing test

---------

Co-authored-by: RichardWaiteSTFC <55979119+RichardWaiteSTFC@users.noreply.github.com>
Co-authored-by: Mohamed Almaki <MohamedAlmaki@users.noreply.github.com>
Co-authored-by: Mohammed Almakki <mohammed.almakki@stfc.ac.uk>
Co-authored-by: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Haigh <35813666+jhaigh0@users.noreply.github.com>
Co-authored-by: James Clarke <james.clarke@stfc.ac.uk>
Co-authored-by: thomashampson <thomas.hampson@stfc.ac.uk>
Co-authored-by: James Clarke <139879523+jclarkeSTFC@users.noreply.github.com>
Co-authored-by: sf1919 <sarah.foxley@stfc.ac.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions Maintenance Unassigned issues to be addressed in the next maintenance period.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants