[R Sharp (r-sharp)]

PR Summary

UMDP3 checker: Allows specification of a single file to check on the command line

Sci/Tech Reviewer: assumed trivial
Code Reviewer: Jenny Hickson (@jennyhickson)

Simpler than expected change to allow the specification of a single file on the command line to run the UMDP3 checker on that file

• closes UMDP3 checker can't be passed a single file to check #221

Code Quality Checklist

• I have performed a self-review of my own code
• My code follows the project's style guidelines
• Comments have been included that aid understanding and enhance the readability of the code
• My changes generate no new warnings
• All automated checks in the CI pipeline have completed successfully

Testing

• This change has been tested appropriately (please describe)

Test has been run passing either a single filename on the command line, or a directory name with and without the --fullcheck option. All 3 runs behaved as expected.
ruff format didn't object to the new lines (for a change)
pytest successfully ran all checks present.

Security Considerations

• I have reviewed my changes for potential security issues
• Sensitive data is properly handled (if applicable)
• Authentication and authorisation are properly implemented (if applicable)

AI Assistance and Attribution

• Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

Sci/Tech Review

• I understand this area of code and the changes being added
• The proposed changes correspond to the pull request description
• Documentation is sufficient (do documentation papers need updating)
• Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

• All dependencies have been resolved
• Related Issues have been properly linked and addressed
• Code quality standards have been met
• Tests are adequate and have passed
• Security considerations have been addressed
• Performance impact is acceptable

[Jenny Hickson (jennyhickson)]

I think this does what it says on the tin, assuming that you're happy to only check a single file in both full-check and CMS modes, rather than just in full-check mode?

[R Sharp (r-sharp)]

I think this does what it says on the tin, assuming that you're happy to only check a single file in both full-check and CMS modes, rather than just in full-check mode?

hmmmm, that's the reverse of the direction I came from.
Originally the change was inside the true side of the if full-check test and in testing it I decided it was a faff to have to add --fullcheck to the command line - hence moving it out and making it a separate first test.

I think combining a single file with a CMS check would be be a bit surplus to requirements. The CMS mode is basically to allow it to automagically create a list of files to check from those the CMS records as changed. Combining the two would be the equivalent of "check this file, but only if the CMS says I've changed it" - without a reasonable amount of extra logic, it would just return "0 files checked, 0 files failed" if you hadn't changed the file. So the assumption is that if you've asked it to look at a single file, it just does that, regardless of what the CMS says about it's changed status.

The (my) assumption is that if you want to focus the check on a single file, it's probably because you want the verbose output (in isolation to all the output from all the files) to create a ToDo list of things to fix.

[Jenny Hickson (jennyhickson)]

I think this does what it says on the tin, assuming that you're happy to only check a single file in both full-check and CMS modes, rather than just in full-check mode?

hmmmm, that's the reverse of the direction I came from. Originally the change was inside the true side of the if full-check test and in testing it I decided it was a faff to have to add --fullcheck to the command line - hence moving it out and making it a separate first test.

I think combining a single file with a CMS check would be be a bit surplus to requirements. The CMS mode is basically to allow it to automagically create a list of files to check from those the CMS records as changed. Combining the two would be the equivalent of "check this file, but only if the CMS says I've changed it" - without a reasonable amount of extra logic, it would just return "0 files checked, 0 files failed" if you hadn't changed the file. So the assumption is that if you've asked it to look at a single file, it just does that, regardless of what the CMS says about it's changed status.

The (my) assumption is that if you want to focus the check on a single file, it's probably because you want the verbose output (in isolation to all the output from all the files) to create a ToDo list of things to fix.

Those seem like fair assumptions and logical conclusions 😸